From f9b7100fc6c77aff92dc45e44beb87539281a7f0 Mon Sep 17 00:00:00 2001 From: Masoud Soroush Date: Sun, 9 Aug 2026 16:18:40 +0200 Subject: [PATCH 1/6] chore(CI): factor the repeated job setup into a composite action - close #330 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight CI jobs opened with the same four steps, so bumping the pnpm pin or changing the store cache was eight identical edits with one file always at risk of being left behind. `.github/actions/setup` now holds pnpm, Node and the install, and every job calls it. The checkout stays in the job. A local action is resolved from the working tree, so `uses: ./.github/actions/setup` cannot carry the checkout that puts it there — and checkout is the step that varies anyway, since ci-web.yml's `web` job needs `fetch-depth: 0` for Codecov base detection. Per-area caches stay in their jobs, moved just below the call: nothing writes the Playwright path during an install, and the Electron binary is fetched by the explicit `rebuild electron` step because the shared install runs --ignore-scripts. Both restores only have to precede the steps that read them. The install passes the manager and its flags through env and a fixed dispatch rather than splicing them into the run body — the same shape ci-editor.yml already uses for the Electron rebuild. Change detection had to learn about the new file. `buildFilters()` discovered CI files by scanning .github/workflows only, so a change to the action alone matched no filter key: every area job skipped and ci-ok green, on a change that alters how every job installs. It now goes through the `wf__` channel with its own `# ci:validates all` marker, which re-runs every job while leaving `changes.root` false — a CI edit must not deploy the site and both workers. Verified against the previous script: the only differences are the added filter key and a log line that now prints a path, since not every entry is `.yml`. --- .claude/skills/ci-cd/SKILL.md | 1 + .github/actions/setup/action.yml | 52 +++++++++++++++++++ .github/workflows/README.md | 6 +++ .github/workflows/ci-editor.yml | 39 +++++--------- .github/workflows/ci-packages.yml | 37 ++++---------- .github/workflows/ci-web.yml | 40 ++++++--------- .github/workflows/ci-worker.yml | 18 ++----- .github/workflows/ci.md | 84 +++++++++++++++++++++---------- .github/workflows/ci.yml | 18 ++----- scripts/assemble-changes.mjs | 37 ++++++++++---- 10 files changed, 192 insertions(+), 140 deletions(-) create mode 100644 .github/actions/setup/action.yml diff --git a/.claude/skills/ci-cd/SKILL.md b/.claude/skills/ci-cd/SKILL.md index ad964744..b355e49c 100644 --- a/.claude/skills/ci-cd/SKILL.md +++ b/.claude/skills/ci-cd/SKILL.md @@ -40,6 +40,7 @@ Own-org used to sit with `actions/*` on a tag. It does not any more: what SHA-pi `prepare` → `lint` → three **caller jobs** (`packages`, `worker`, `app`) → `ci-ok`. Each caller `uses:` an area workflow; `ci-app.yml` calls one workflow per app in turn, so adding an app never touches the entry file. Nesting is three of the four levels GitHub allows, and it stays one run with one `ci-ok`. - **Detect once in `prepare`** (node version from `.nvmrc`, package manager, runner, changed areas), reuse via `needs.prepare.outputs.*`. Never hard-code the node version. +- **Every job that installs starts from `./.github/actions/setup`** — the composite action holding pnpm, Node and the install. **Never re-inline those steps**: the whole point is that the `pnpm/action-setup` pin and the store cache are a one-file edit. A new job is a checkout (the action cannot carry it — a local action is resolved from the working tree it checks out) plus a call with `node_version`/`manager`/`command`. Per-area caches (Playwright binaries, the Electron binary) stay in the job, just after the call. The action carries its own `# ci:validates all` marker, so editing it re-runs every job without setting `changes.root` — nothing deploys off a CI edit. - **A file per area, so the gate can be narrower than everything.** Each workflow declares its scope on line 1 (`# ci:validates pkg__*`), read by `scripts/assemble-changes.mjs`; unmarked or unparseable means the whole workspace. A caller job cannot set `environment:`/`timeout-minutes:`/`runs-on:` (those belong to the inner jobs), and **`secrets: inherit` is mandatory, per hop** — naming an environment-scoped secret at the call site passes an empty string, and a middle layer that omits it starves the workflow below. - **One job per shape, not per member.** Packages, workers and the editor's unit tier are the same job — install, `test:coverage`, upload the lcov — so packages are one matrix and workers another, both built from the tree in `scripts/assemble-changes.mjs`. **Adding a workspace member must need no edit to `ci.yml`**: if a new area needs a job, ask first whether it is really a different shape (`web` builds; `editor-e2e` drives Electron) or just another row. - Heavy jobs are **change-gated** (`dorny/paths-filter`, no Nx/Turbo) so a package-only PR stays cheap. Dependency edges are **derived, never listed**: a member runs when it changed or when a package it declares as a `workspace:` dependency changed. Do not add a hand-written consumer list — it is a list to forget the day a dependency moves. diff --git a/.github/actions/setup/action.yml b/.github/actions/setup/action.yml new file mode 100644 index 00000000..4936efb3 --- /dev/null +++ b/.github/actions/setup/action.yml @@ -0,0 +1,52 @@ +# ci:validates all +# Every job installs through this action, so editing it re-runs every one of them — and, being a +# CI file rather than a root file, ships nothing. +# +# The four steps every CI job used to repeat, minus the checkout: a local action is +# resolved from the working tree, so `uses: ./.github/actions/setup` cannot carry the +# checkout that puts it there. Jobs keep their own — which is also the step that varies +# (ci-web.yml's `web` job needs `fetch-depth: 0` for Codecov base detection). +# +# Per-area caches stay in the jobs that need them. This holds only what every job shares, +# so bumping the pnpm pin or changing the store cache is an edit to this file alone. +name: Setup +description: Set up pnpm and Node, then install the workspace's dependencies. + +inputs: + node_version: + description: Node.js version to install — ci.yml's `prepare` job reads it from .nvmrc. + required: true + manager: + description: Detected package manager — pnpm, yarn or npm. + required: true + command: + description: The manager's install command, with its flags. + required: true + +runs: + using: composite + steps: + - name: Setup pnpm + if: inputs.manager == 'pnpm' + uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 + + - name: Setup Node + uses: actions/setup-node@v5 + with: + node-version: ${{ inputs.node_version }} + cache: ${{ inputs.manager }} + + # Both inputs reach the shell through env and a fixed dispatch, never spliced into + # the run body — zizmor: template-injection. `$COMMAND` is deliberately unquoted: it + # is a flag list ("install --frozen-lockfile --ignore-scripts"), not a single word. + - name: Install dependencies + shell: bash + env: + CI: true + MANAGER: ${{ inputs.manager }} + COMMAND: ${{ inputs.command }} + run: | + case "$MANAGER" in + pnpm|yarn|npm) "$MANAGER" $COMMAND ;; + *) echo "Unsupported package manager: $MANAGER" >&2; exit 1 ;; + esac diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 494dbffa..9080f6d3 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -23,6 +23,12 @@ raw `push`; package publishing (`cd-packages`) and the editor release (`cd-edito | [`chromatic.yml`](./chromatic.yml) | Chromatic | `push` to `main` (paths) + `workflow_dispatch` | | [`label-area.yml`](./label-area.yml) | Label Affected Area | `issues` `opened` | +Everything shared by the jobs that install — pnpm, Node, the install itself — is the composite +action [`.github/actions/setup`](../actions/setup/action.yml), called by every one of them. +**A new job starts from a checkout and a call to it**, so bumping the `pnpm/action-setup` pin +or changing the store cache is an edit to one file: see +[the setup action](./ci.md#the-setup-action). + **Per-workflow deep dives** (every step + caching): [`ci.md`](./ci.md) · [`ci-packages.md`](./ci-packages.md) · [`ci-worker.md`](./ci-worker.md) · [`ci-app.md`](./ci-app.md) · [`ci-web.md`](./ci-web.md) · [`ci-editor.md`](./ci-editor.md) · [`cd-web.md`](./cd-web.md) · [`cd-worker-api.md`](./cd-worker-api.md) · [`cd-worker-bench.md`](./cd-worker-bench.md) · [`cd-packages.md`](./cd-packages.md) · [`cd-editor.md`](./cd-editor.md) · [`chromatic.md`](./chromatic.md) · [`label-area.md`](./label-area.md) diff --git a/.github/workflows/ci-editor.yml b/.github/workflows/ci-editor.yml index 1e30e45c..328d6661 100644 --- a/.github/workflows/ci-editor.yml +++ b/.github/workflows/ci-editor.yml @@ -33,30 +33,23 @@ jobs: with: persist-credentials: false - - name: Setup pnpm - if: inputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ inputs.node_version }} - cache: ${{ inputs.manager }} + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} # Electron's postinstall downloads its binary to ~/.cache/electron, outside the store # setup-node caches. Both jobs install, so both want it. Keyed on the manifest, so an - # upgrade fetches and nothing else does. + # upgrade fetches and nothing else does. It sits after the install because the install + # runs --ignore-scripts: the binary is fetched by the explicit rebuild step below. - name: Restore cache - Electron binary uses: actions/cache@v5 with: path: ~/.cache/electron key: ${{ runner.os }}-electron-${{ hashFiles('apps/editor/package.json') }} - - name: Install dependencies - env: - CI: true - run: ${{ inputs.manager }} ${{ inputs.command }} - # The shared install command runs --ignore-scripts, which also skips # Electron's postinstall — the binary download. Rebuild runs just that one # script, explicitly: into ~/.cache/electron (restored above), unpacked to @@ -104,15 +97,12 @@ jobs: with: persist-credentials: false - - name: Setup pnpm - if: inputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ inputs.node_version }} - cache: ${{ inputs.manager }} + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} - name: Restore cache - Electron binary uses: actions/cache@v5 @@ -120,11 +110,6 @@ jobs: path: ~/.cache/electron key: ${{ runner.os }}-electron-${{ hashFiles('apps/editor/package.json') }} - - name: Install dependencies - env: - CI: true - run: ${{ inputs.manager }} ${{ inputs.command }} - # Same as the unit job: the install skipped Electron's postinstall, so run # just that one script — this tier launches the real binary. Same fixed # dispatch as there, keeping the input out of shell source. diff --git a/.github/workflows/ci-packages.yml b/.github/workflows/ci-packages.yml index 684f2218..df99f192 100644 --- a/.github/workflows/ci-packages.yml +++ b/.github/workflows/ci-packages.yml @@ -35,16 +35,14 @@ jobs: with: persist-credentials: false - - name: Setup pnpm - if: inputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ inputs.node_version }} - cache: ${{ inputs.manager }} + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} + # As in ci-web.yml: after the install, which touches nothing on this path. - name: Restore cache - Playwright binaries if: matrix.browsers uses: actions/cache/restore@v5 @@ -53,11 +51,6 @@ jobs: path: ${{ github.workspace }}/ms-playwright key: ${{ runner.os }}-playwright-${{ inputs.playwright_version }} - - name: Install dependencies - env: - CI: true - run: ${{ inputs.manager }} ${{ inputs.command }} - # Only Chromium is needed — the browser test tier runs headless Chromium. - name: Install Playwright browsers if: matrix.browsers && steps.playwright-cache.outputs.cache-hit != 'true' @@ -128,20 +121,12 @@ jobs: with: persist-credentials: false - - name: Setup pnpm - if: inputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ inputs.node_version }} - cache: ${{ inputs.manager }} - - - name: Install dependencies - env: - CI: true - run: ${{ inputs.manager }} ${{ inputs.command }} + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} # The bench file imports this package's built dist so the comparison # against the npm release's prebuilt dist is like-for-like. diff --git a/.github/workflows/ci-web.yml b/.github/workflows/ci-web.yml index 419fa8c9..8bd9cc0d 100644 --- a/.github/workflows/ci-web.yml +++ b/.github/workflows/ci-web.yml @@ -37,16 +37,15 @@ jobs: fetch-depth: 0 # full history for Codecov base detection persist-credentials: false - - name: Setup pnpm - if: inputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ inputs.node_version }} - cache: ${{ inputs.manager }} + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} + # After the install rather than before it: nothing writes this path during install, + # and the restore only has to precede the browser-install and save steps below. - name: Restore cache - Playwright binaries uses: actions/cache/restore@v5 id: playwright-cache @@ -54,11 +53,6 @@ jobs: path: ${{ github.workspace }}/ms-playwright key: ${{ runner.os }}-playwright-${{ inputs.playwright_version }} - - name: Install dependencies - env: - CI: true - run: ${{ inputs.manager }} ${{ inputs.command }} - # Only Chromium is needed here — the unit-browser and storybook tiers run headless Chromium. - name: Install Playwright browsers if: steps.playwright-cache.outputs.cache-hit != 'true' @@ -186,16 +180,15 @@ jobs: with: persist-credentials: false - - name: Setup pnpm - if: inputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ inputs.node_version }} - cache: ${{ inputs.manager }} + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} + # As in the `web` job: restoring after the install is equivalent, and it leaves the + # shared setup in one piece. - name: Restore cache - Playwright binaries uses: actions/cache/restore@v5 id: playwright-cache @@ -203,11 +196,6 @@ jobs: path: ${{ github.workspace }}/ms-playwright key: ${{ runner.os }}-playwright-${{ inputs.playwright_version }} - - name: Install dependencies - env: - CI: true - run: ${{ inputs.manager }} ${{ inputs.command }} - - name: Install Playwright browsers if: steps.playwright-cache.outputs.cache-hit != 'true' run: pnpm --filter @soroush/web exec playwright install --with-deps ${{ matrix.engine }} diff --git a/.github/workflows/ci-worker.yml b/.github/workflows/ci-worker.yml index 03ab0937..45cfcb3e 100644 --- a/.github/workflows/ci-worker.yml +++ b/.github/workflows/ci-worker.yml @@ -30,20 +30,12 @@ jobs: with: persist-credentials: false - - name: Setup pnpm - if: inputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ inputs.node_version }} - cache: ${{ inputs.manager }} - - - name: Install dependencies - env: - CI: true - run: ${{ inputs.manager }} ${{ inputs.command }} + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} - name: Run Tests with Coverage run: ${{ inputs.runner }} --filter ${{ matrix.filter }} test:coverage diff --git a/.github/workflows/ci.md b/.github/workflows/ci.md index 473b17ac..f504f957 100644 --- a/.github/workflows/ci.md +++ b/.github/workflows/ci.md @@ -58,17 +58,17 @@ and a one-line change to how the editor runs its tests re-runs the editor. See `runs-on: ubuntu-latest` · `timeout-minutes: 15`. Produces every output the other jobs consume via `needs.prepare.outputs.*`. -| # | Step | Run / Action | What it does | -| --- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | Checkout Repository | `actions/checkout@v5` (`persist-credentials: false`) | Clone the repo without leaving the token on disk. | -| 2 | Read Node.js version | `cat .nvmrc` → `$GITHUB_OUTPUT` | Single source of truth for the Node version; never hard-coded. | -| 3 | Detect package manager | shell `if` on lockfile presence | Emits `manager` (`pnpm`/`yarn`/`npm`), `command` (e.g. `install --frozen-lockfile --ignore-scripts` — no lifecycle script runs at install anywhere in CI), `runner`. Fails if none found. | -| 4 | Read Playwright version | `node -p "...devDependencies?.['@playwright/test'] \|\| ...dependencies?.['@playwright/test']"` then strip leading `^` | Feeds the Playwright binary cache key. **Must read `@playwright/test`** — the project has no bare `playwright` dep, so reading `playwright` yields the string `"undefined"` and freezes the cache key (see [Caching](#caching)). | -| 5 | Discover workspace entities | `node scripts/assemble-changes.mjs filters` | Builds a per-entity `paths-filter` config: one key per app (`app__`), worker (`worker__`), package (`pkg__`), workflow file (`wf__`), and **one per root file** (`root__lock`, `root__workspace`, `root__package`, `root__nvmrc`, `root__tsconfig`) — a whitelist, so root docs/tooling dotfiles trigger nothing. | -| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched. | -| 7 | Copy base manifest + lockfile | `git show "$BASE_SHA:…"` into `$RUNNER_TEMP/base`, fetching one commit deep first if the shallow clone lacks it | Hands the next step the two files it compares against. Every command may fail without failing the job — a missing or empty copy reads as "cannot be compared", which validates everything. | -| 8 | Assemble `changes.json` | `node scripts/assemble-changes.mjs assemble` | Writes [`changes.json`](#changesjson) (the lists + `root`), and derives this run's own gating outputs `web` / `editor` / `has_packages` / `changed_packages` / `has_workers` / `changed_workers`. Root files are attributed first — see [What a root change means](#what-a-root-change-means). A whole-workspace root file, or a change to `ci.yml` itself, means every job runs. | -| 9 | Upload `changes.json` | `actions/upload-artifact@v7` (name `changes`) | Hands the single file to the CD workflows, which run on `workflow_run` and have no diff base of their own. | +| # | Step | Run / Action | What it does | +| --- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | Checkout Repository | `actions/checkout@v5` (`persist-credentials: false`) | Clone the repo without leaving the token on disk. | +| 2 | Read Node.js version | `cat .nvmrc` → `$GITHUB_OUTPUT` | Single source of truth for the Node version; never hard-coded. | +| 3 | Detect package manager | shell `if` on lockfile presence | Emits `manager` (`pnpm`/`yarn`/`npm`), `command` (e.g. `install --frozen-lockfile --ignore-scripts` — no lifecycle script runs at install anywhere in CI), `runner`. Fails if none found. | +| 4 | Read Playwright version | `node -p "...devDependencies?.['@playwright/test'] \|\| ...dependencies?.['@playwright/test']"` then strip leading `^` | Feeds the Playwright binary cache key. **Must read `@playwright/test`** — the project has no bare `playwright` dep, so reading `playwright` yields the string `"undefined"` and freezes the cache key (see [Caching](#caching)). | +| 5 | Discover workspace entities | `node scripts/assemble-changes.mjs filters` | Builds a per-entity `paths-filter` config: one key per app (`app__`), worker (`worker__`), package (`pkg__`), CI definition file (`wf__` — every workflow, plus `wf__actions-setup` for the [setup action](#the-setup-action)), and **one per root file** (`root__lock`, `root__workspace`, `root__package`, `root__nvmrc`, `root__tsconfig`) — a whitelist, so root docs/tooling dotfiles trigger nothing. | +| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched. | +| 7 | Copy base manifest + lockfile | `git show "$BASE_SHA:…"` into `$RUNNER_TEMP/base`, fetching one commit deep first if the shallow clone lacks it | Hands the next step the two files it compares against. Every command may fail without failing the job — a missing or empty copy reads as "cannot be compared", which validates everything. | +| 8 | Assemble `changes.json` | `node scripts/assemble-changes.mjs assemble` | Writes [`changes.json`](#changesjson) (the lists + `root`), and derives this run's own gating outputs `web` / `editor` / `has_packages` / `changed_packages` / `has_workers` / `changed_workers`. Root files are attributed first — see [What a root change means](#what-a-root-change-means). A whole-workspace root file, or a change to `ci.yml` itself, means every job runs. | +| 9 | Upload `changes.json` | `actions/upload-artifact@v7` (name `changes`) | Hands the single file to the CD workflows, which run on `workflow_run` and have no diff base of their own. | ### Outputs @@ -155,14 +155,12 @@ the resolved graph, and getting it wrong means skipping a package the bump did b `needs: prepare` · ubuntu · 15 min. Lint + typecheck the whole workspace once (`pnpm -r` skips workspaces without the script). Not change-gated — it's cheap. -| # | Step | Detail | -| --- | ---------- | -------------------------------------------------------------------------------------------------------------------- | -| 1 | Checkout | `actions/checkout@v5`, no persisted creds | -| 2 | Setup pnpm | `pnpm/action-setup@v5`, only `if manager == 'pnpm'` | -| 3 | Setup Node | `actions/setup-node@v5` with `node-version: ` and `cache: ` (deps cache — see [Caching](#caching)) | -| 4 | Install | `${manager} ${command}` with `CI: true` | -| 5 | Lint | `${runner} run lint` (`--max-warnings 0`) | -| 6 | Typecheck | `${runner} run typecheck` | +| # | Step | Detail | +| --- | --------- | ---------------------------------------------------------------------------------------- | +| 1 | Checkout | `actions/checkout@v5`, no persisted creds | +| 2 | Setup | [`./.github/actions/setup`](#the-setup-action) — pnpm, Node and the install, in one step | +| 3 | Lint | `${runner} run lint` (`--max-warnings 0`) | +| 4 | Typecheck | `${runner} run typecheck` | --- @@ -191,9 +189,43 @@ Three things about a caller job that are easy to get wrong: - Check names become `caller / called-job` (`web / e2e (chromium)`). Only `ci-ok` is required, and it stays a top-level job here, so branch protection is unaffected. +### The setup action + +Every job that installs starts the same way, so the shared part lives in one composite action — +[`.github/actions/setup`](../actions/setup/action.yml): + +```yaml +- name: Checkout Repository + uses: actions/checkout@v5 + with: + persist-credentials: false + +- name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup + with: + node_version: ${{ inputs.node_version }} + manager: ${{ inputs.manager }} + command: ${{ inputs.command }} +``` + +**Start a new job from that block.** Bumping the `pnpm/action-setup` pin or changing the store +cache is then an edit to one file, rather than one per job with a file left behind. + +Two things it deliberately does not do: + +- **It does not check out.** A local action is resolved from the working tree, so the checkout that + puts it there cannot live inside it. The job keeps its own — which is also the step that varies: + `ci-web.yml`'s `web` job needs `fetch-depth: 0` for Codecov base detection. +- **It does not cache per area.** Playwright binaries and the Electron binary stay in the jobs that + want them, restored just after the call — nothing writes those paths during an install, and the + restores only have to precede the steps that read them. + +It carries a `# ci:validates all` marker of its own, so editing it re-runs every job. That is the +CI-side `all` and not a root change: `changes.root` stays false, and nothing deploys. + ### What a workflow validates -Every workflow declares its own scope on line 1, read by +Every workflow — and the setup action — declares its own scope on line 1, read by [`scripts/assemble-changes.mjs`](../../scripts/assemble-changes.mjs): ```yaml @@ -240,23 +272,23 @@ Two independent caches; both are keyed so a real change busts them. ### 1. Dependency store (`actions/setup-node`) -Every job that installs uses `setup-node@v5` with `cache: ${{ … manager }}`. For -pnpm this caches the **pnpm store**, keyed automatically off the `pnpm-lock.yaml` -hash. A lockfile change → new key → fresh install; otherwise the store is restored -and `--frozen-lockfile` just links. +Set once, in [the setup action](#the-setup-action): `setup-node@v5` with +`cache: ${{ inputs.manager }}`. For pnpm this caches the **pnpm store**, keyed +automatically off the `pnpm-lock.yaml` hash. A lockfile change → new key → fresh +install; otherwise the store is restored and `--frozen-lockfile` just links. ### 2. Playwright browser binaries (browser-tier package rows, `web`, `e2e`) ```yaml env: PLAYWRIGHT_BROWSERS_PATH: ${{ github.workspace }}/ms-playwright -# restore (before install) +# restore (after the setup action installed — nothing writes this path during an install) - uses: actions/cache/restore@v5 id: playwright-cache with: path: ${{ github.workspace }}/ms-playwright key: ${{ runner.os }}-playwright-${{ needs.prepare.outputs.playwright_version }} -# … install deps, then on a miss `playwright install --with-deps`, +# … then on a miss `playwright install --with-deps`, # on a hit `playwright install-deps` (Linux only) … # save (only on a miss, even if later steps fail) - uses: actions/cache/save@v5 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 98b97a6c..a69fd4d4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -137,20 +137,12 @@ jobs: with: persist-credentials: false - - name: Setup pnpm - if: needs.prepare.outputs.manager == 'pnpm' - uses: pnpm/action-setup@fc06bc1257f339d1d5d8b3a19a8cae5388b55320 # v5 - - - name: Setup Node - uses: actions/setup-node@v5 + - name: Setup pnpm, Node and dependencies + uses: ./.github/actions/setup with: - node-version: ${{ needs.prepare.outputs.node_version }} - cache: ${{ needs.prepare.outputs.manager }} - - - name: Install dependencies - env: - CI: true - run: ${{ needs.prepare.outputs.manager }} ${{ needs.prepare.outputs.command }} + node_version: ${{ needs.prepare.outputs.node_version }} + manager: ${{ needs.prepare.outputs.manager }} + command: ${{ needs.prepare.outputs.command }} - name: Run Lint run: ${{ needs.prepare.outputs.runner }} run lint diff --git a/scripts/assemble-changes.mjs b/scripts/assemble-changes.mjs index 667a56aa..82e7f0c7 100644 --- a/scripts/assemble-changes.mjs +++ b/scripts/assemble-changes.mjs @@ -50,19 +50,31 @@ const dirsWithPackageJson = (root) => { } /** - * The paths-filter config: one key per workspace member, per workflow file, and per root file. - * JSON is valid YAML, so the action takes this as its `filters` input verbatim. + * Every CI definition file that carries a `# ci:validates` marker, by the name its `wf__` filter + * key uses. The workflows are discovered; the composite setup action is named, being the one such + * file outside `.github/workflows/`. It belongs here rather than in `ROOT_FILES`: every job + * installs through it, so editing it must re-run all of them — but it is a CI file, and a root + * file additionally sets `changes.root`, which is what the CD workflows deploy on. + */ +export function ciFiles() { + const files = {} + for (const file of readdirSync(join(repoRoot, '.github/workflows'))) { + if (/\.ya?ml$/.test(file)) files[file.replace(/\.ya?ml$/, '')] = `.github/workflows/${file}` + } + files['actions-setup'] = '.github/actions/setup/action.yml' + return files +} + +/** + * The paths-filter config: one key per workspace member, per CI definition file, and per root + * file. JSON is valid YAML, so the action takes this as its `filters` input verbatim. */ export function buildFilters() { const filters = {} for (const { dir, prefix } of AREAS) { for (const name of dirsWithPackageJson(dir)) filters[`${prefix}${name}`] = [`${dir}/${name}/**`] } - for (const file of readdirSync(join(repoRoot, '.github/workflows'))) { - if (/\.ya?ml$/.test(file)) { - filters[`wf__${file.replace(/\.ya?ml$/, '')}`] = [`.github/workflows/${file}`] - } - } + for (const [name, file] of Object.entries(ciFiles())) filters[`wf__${name}`] = [file] for (const [key, file] of Object.entries(ROOT_FILES)) filters[key] = [file] return filters } @@ -163,7 +175,12 @@ const MARKER = '# ci:validates ' export function workflowValidates(name) { let source try { - source = readFileSync(join(repoRoot, '.github/workflows', `${name}.yml`), 'utf8') + // Deleted files are gone from `ciFiles()`, so fall back to where a workflow would have been: + // the read then fails, which is the answer either way. + source = readFileSync( + join(repoRoot, ciFiles()[name] ?? `.github/workflows/${name}.yml`), + 'utf8' + ) } catch { // Deleted in this very change: there is nothing left to read, so nothing is claimed. return { keys: [], wholeWorkspace: false } @@ -200,13 +217,15 @@ export function workflowValidates(name) { * whether any of them means the lot. */ export function attributeWorkflows(workflows) { + const files = ciFiles() const claims = workflows.map((name) => ({ name, ...workflowValidates(name) })) return { keys: claims.flatMap(({ keys }) => keys), wholeWorkspace: claims.some((claim) => claim.wholeWorkspace), + // By path rather than by name: not every one of these is `.yml` any more. reasons: claims.map( ({ name, keys, wholeWorkspace }) => - `${name}.yml changed — ${wholeWorkspace ? 'it decides how every job runs' : keys.join(', ') || 'CI never runs it'}` + `${files[name] ?? `${name}.yml`} changed — ${wholeWorkspace ? 'it decides how every job runs' : keys.join(', ') || 'CI never runs it'}` ), } } From 2c8979ef5b56a3dc24ece35790b57df197f5bcc1 Mon Sep 17 00:00:00 2001 From: Masoud Soroush Date: Sun, 9 Aug 2026 16:20:35 +0200 Subject: [PATCH 2/6] chore(CI): name every workflow CI or CD so the Actions list groups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The five area workflows already read `CI · `, but the entry workflow and four of the five deploys did not — `Continuous Integration`, `Continuous Deployment to GitHub Pages`, `Deploy Worker (@soroush/api)`, `Publish Packages (npm)` — so the sidebar was a list to read rather than two blocks to scan. ci.yml Continuous Integration -> CI cd-web.yml Continuous Deployment to GitHub Pages -> CD · Web (Pages + Storybook) cd-worker-api.yml Deploy Worker (@soroush/api) -> CD · Worker (api) cd-worker-bench.yml Deploy Worker (@soroush/bench-api) -> CD · Worker (bench) cd-packages.yml Publish Packages (npm) -> CD · Packages (npm) cd-editor.yml CD · Editor -> CD · Editor (release) Chromatic and the labeller stay unprefixed. Neither is part of ci-ok, and a CI prefix would say they gate a pull request when they do not. Renaming ci.yml is not a one-file edit: `workflow_run` matches on a workflow's `name:`, never its filename, so cd-web, cd-worker-api and cd-worker-bench all move from `workflows: ['Continuous Integration']` to `workflows: ['CI']` in the same change. Miss one and that deploy stops firing without an error. Thirteen doc references move with them, including the cd-packages runbook step that names the entry to click in the Actions UI. Branch protection is unaffected — it matches the job name `ci-ok`. ci.yml's concurrency group is keyed on `github.workflow`, so its value changes; the group stays internally consistent either side of the rename. The naming rule and the workflow_run coupling are written down in the workflows README and the ci-cd skill, so the next rename starts from them. --- .claude/skills/ci-cd/SKILL.md | 30 +++++++------- .claude/skills/release-notes/SKILL.md | 2 +- .github/workflows/README.md | 56 +++++++++++++++------------ .github/workflows/cd-editor.md | 2 +- .github/workflows/cd-editor.yml | 2 +- .github/workflows/cd-packages.md | 4 +- .github/workflows/cd-packages.yml | 2 +- .github/workflows/cd-web.md | 6 +-- .github/workflows/cd-web.yml | 4 +- .github/workflows/cd-worker-api.md | 6 +-- .github/workflows/cd-worker-api.yml | 4 +- .github/workflows/cd-worker-bench.md | 4 +- .github/workflows/cd-worker-bench.yml | 4 +- .github/workflows/ci.md | 2 +- .github/workflows/ci.yml | 2 +- 15 files changed, 69 insertions(+), 61 deletions(-) diff --git a/.claude/skills/ci-cd/SKILL.md b/.claude/skills/ci-cd/SKILL.md index b355e49c..467de1bd 100644 --- a/.claude/skills/ci-cd/SKILL.md +++ b/.claude/skills/ci-cd/SKILL.md @@ -9,23 +9,25 @@ Each workflow has a per-file deep-dive doc next to it (`ci.md`, `cd-*.md`, `chro ## Workflow files -| File | Name | Trigger | -| ------------------- | ------------------------ | ---------------------------------------------------------------------------- | -| `ci.yml` | `Continuous Integration` | `push` to `main`, all `pull_request` | -| `ci-packages.yml` | CI · Packages | `workflow_call` from `ci.yml` | -| `ci-worker.yml` | CI · Workers | `workflow_call` from `ci.yml` | -| `ci-app.yml` | CI · Apps | `workflow_call` from `ci.yml` | -| `ci-web.yml` | CI · Web | `workflow_call` from `ci-app.yml` | -| `ci-editor.yml` | CI · Editor | `workflow_call` from `ci-app.yml` | -| `cd-web.yml` | Pages + Storybook deploy | `workflow_run` of CI (success, `main`) + dispatch | -| `cd-worker-api.yml` | Cloudflare Worker deploy | `workflow_run` of CI (success, `main`) + dispatch | -| `cd-packages.yml` | Publish Packages (npm) | manual `workflow_dispatch` only — see the `release-notes` skill | -| `cd-editor.yml` | CD · Editor | manual `workflow_dispatch` only — draft GitHub Release of the installers | -| `chromatic.yml` | Chromatic | `pull_request` + `push` to `main` + `workflow_dispatch` (main), non-blocking | -| `label-area.yml` | Label Affected Area | `issues: opened` | +| File | Name | Trigger | +| ------------------- | ------------------------------ | ---------------------------------------------------------------------------- | +| `ci.yml` | `CI` | `push` to `main`, all `pull_request` | +| `ci-packages.yml` | `CI · Packages` | `workflow_call` from `ci.yml` | +| `ci-worker.yml` | `CI · Workers` | `workflow_call` from `ci.yml` | +| `ci-app.yml` | `CI · Apps` | `workflow_call` from `ci.yml` | +| `ci-web.yml` | `CI · Web` | `workflow_call` from `ci-app.yml` | +| `ci-editor.yml` | `CI · Editor` | `workflow_call` from `ci-app.yml` | +| `cd-web.yml` | `CD · Web (Pages + Storybook)` | `workflow_run` of CI (success, `main`) + dispatch | +| `cd-worker-api.yml` | `CD · Worker (api)` | `workflow_run` of CI (success, `main`) + dispatch | +| `cd-packages.yml` | `CD · Packages (npm)` | manual `workflow_dispatch` only — see the `release-notes` skill | +| `cd-editor.yml` | `CD · Editor (release)` | manual `workflow_dispatch` only — draft GitHub Release of the installers | +| `chromatic.yml` | `Chromatic` | `pull_request` + `push` to `main` + `workflow_dispatch` (main), non-blocking | +| `label-area.yml` | `Label Affected Area` | `issues: opened` | One CI entry workflow calling one per area; CD is separate and **gated on CI success** — never deploy on a raw `push`. +**Naming.** Every workflow is `CI · ` or `CD · ()`, so the Actions sidebar groups into two blocks; the entry workflow is plain `CI`. Chromatic and the labeller stay **unprefixed on purpose** — neither is part of `ci-ok`, and prefixing them would say they gate PRs. **Renaming a workflow is never a one-file edit**: `workflow_run` matches on the workflow's `name:`, not its filename, so `cd-web` / `cd-worker-api` / `cd-worker-bench` all pin `workflows: ['CI']` and a rename that misses one silently stops that deploy for good. Branch protection is unaffected — it matches the **job** name `ci-ok`. + ## Action pinning convention — the load-bearing rule Pin every `uses:` by the action's **origin**. Getting this wrong fails review: CodeRabbit flags SHA-pinned `actions/*`; SonarQube flags anything else on a version tag. diff --git a/.claude/skills/release-notes/SKILL.md b/.claude/skills/release-notes/SKILL.md index 9e70a754..98e179ab 100644 --- a/.claude/skills/release-notes/SKILL.md +++ b/.claude/skills/release-notes/SKILL.md @@ -133,7 +133,7 @@ Maintenance release — dependency refresh only. No public API or behavior chang step fails again before publishing. That same check also fails a commit whose **staged** package isn't ahead of the version already on npm, so an edit can't land without its bump. A package you didn't touch may stay at its published version. -2. Dispatch — Actions → **Publish Packages (npm)** → pick `package`, **Run**. CLI: +2. Dispatch — Actions → **CD · Packages (npm)** → pick `package`, **Run**. CLI: `gh workflow run cd-packages.yml -f package=`. 3. The job publishes to npm (skips if that version already exists) and cuts a GitHub Release tagged/titled `@` from the notes file. Re-running repairs a missing Release diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 9080f6d3..97cfd558 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -7,21 +7,27 @@ Chromatic visual-review workflow, and an issue-labeling automation. The three de raw `push`; package publishing (`cd-packages`) and the editor release (`cd-editor`) are **manual `workflow_dispatch` only**. -| File | Name | Trigger | -| ---------------------------------------------- | ------------------------- | ------------------------------------------------------------ | -| [`ci.yml`](./ci.yml) | `Continuous Integration` | `push` to `main`, every `pull_request` | -| [`ci-packages.yml`](./ci-packages.yml) | CI · Packages | `workflow_call` from `ci.yml` | -| [`ci-worker.yml`](./ci-worker.yml) | CI · Workers | `workflow_call` from `ci.yml` | -| [`ci-app.yml`](./ci-app.yml) | CI · Apps | `workflow_call` from `ci.yml` | -| [`ci-web.yml`](./ci-web.yml) | CI · Web | `workflow_call` from `ci-app.yml` | -| [`ci-editor.yml`](./ci-editor.yml) | CI · Editor | `workflow_call` from `ci-app.yml` | -| [`cd-web.yml`](./cd-web.yml) | Pages + Storybook deploy | `workflow_run` of CI (success, `main`) + `workflow_dispatch` | -| [`cd-worker-api.yml`](./cd-worker-api.yml) | Cloudflare Worker deploy | `workflow_run` of CI (success, `main`) + `workflow_dispatch` | -| [`cd-worker-bench.yml`](./cd-worker-bench.yml) | Bench relay Worker deploy | `workflow_run` of CI (success, `main`) + `workflow_dispatch` | -| [`cd-packages.yml`](./cd-packages.yml) | Publish Packages (npm) | manual `workflow_dispatch` only | -| [`cd-editor.yml`](./cd-editor.yml) | CD · Editor | manual `workflow_dispatch` only | -| [`chromatic.yml`](./chromatic.yml) | Chromatic | `push` to `main` (paths) + `workflow_dispatch` | -| [`label-area.yml`](./label-area.yml) | Label Affected Area | `issues` `opened` | +| File | Name | Trigger | +| ---------------------------------------------- | ------------------------------ | ------------------------------------------------------------ | +| [`ci.yml`](./ci.yml) | `CI` | `push` to `main`, every `pull_request` | +| [`ci-packages.yml`](./ci-packages.yml) | `CI · Packages` | `workflow_call` from `ci.yml` | +| [`ci-worker.yml`](./ci-worker.yml) | `CI · Workers` | `workflow_call` from `ci.yml` | +| [`ci-app.yml`](./ci-app.yml) | `CI · Apps` | `workflow_call` from `ci.yml` | +| [`ci-web.yml`](./ci-web.yml) | `CI · Web` | `workflow_call` from `ci-app.yml` | +| [`ci-editor.yml`](./ci-editor.yml) | `CI · Editor` | `workflow_call` from `ci-app.yml` | +| [`cd-web.yml`](./cd-web.yml) | `CD · Web (Pages + Storybook)` | `workflow_run` of CI (success, `main`) + `workflow_dispatch` | +| [`cd-worker-api.yml`](./cd-worker-api.yml) | `CD · Worker (api)` | `workflow_run` of CI (success, `main`) + `workflow_dispatch` | +| [`cd-worker-bench.yml`](./cd-worker-bench.yml) | `CD · Worker (bench)` | `workflow_run` of CI (success, `main`) + `workflow_dispatch` | +| [`cd-packages.yml`](./cd-packages.yml) | `CD · Packages (npm)` | manual `workflow_dispatch` only | +| [`cd-editor.yml`](./cd-editor.yml) | `CD · Editor (release)` | manual `workflow_dispatch` only | +| [`chromatic.yml`](./chromatic.yml) | `Chromatic` | `push` to `main` (paths) + `workflow_dispatch` | +| [`label-area.yml`](./label-area.yml) | `Label Affected Area` | `issues` `opened` | + +Names are `CI · ` and `CD · `, so the Actions sidebar groups into two blocks — the +entry workflow is plain `CI`. Chromatic and the labeller are unprefixed because neither is part of +`ci-ok`. **Renaming one is never a one-file edit**: `workflow_run` matches on the `name:`, not the +filename, so the three deploys pin `workflows: ['CI']` and a rename that misses one stops that +deploy without a word. Branch protection matches the job name `ci-ok`, so it is unaffected. Everything shared by the jobs that install — pnpm, Node, the install itself — is the composite action [`.github/actions/setup`](../actions/setup/action.yml), called by every one of them. @@ -42,19 +48,19 @@ artifact. ```mermaid flowchart LR - push["push to main"] --> ci["CI — Continuous Integration"] + push["push to main"] --> ci["CI"] pr["pull_request"] --> ci ci -->|"uploads artifact"| art[("changes.json
apps · worker · packages
workflows · root")] - ci -->|"workflow_run: completed + success on main"| cdweb["CD — Pages"] - ci -->|"workflow_run: completed + success on main"| cdworker["CD — Worker API"] - ci -->|"workflow_run: completed + success on main"| cdbench["CD — Bench relay Worker"] + ci -->|"workflow_run: completed + success on main"| cdweb["CD · Web"] + ci -->|"workflow_run: completed + success on main"| cdworker["CD · Worker (api)"] + ci -->|"workflow_run: completed + success on main"| cdbench["CD · Worker (bench)"] art -.->|"download-artifact"| cdweb art -.->|"download-artifact"| cdworker art -.->|"download-artifact"| cdbench cdweb --> pages["GitHub Pages"] cdworker --> cf["Cloudflare Worker"] cdbench --> cfbench["Cloudflare Worker (bench relay)"] - disp["workflow_dispatch (manual)"] --> cdpkg["CD — Packages"] + disp["workflow_dispatch (manual)"] --> cdpkg["CD · Packages"] cdpkg --> npm["npm registry"] ``` @@ -66,7 +72,7 @@ deploys on `apps`/`packages`/`root`); the policy lives in CD, the facts in CI. I artifact is missing (e.g. a manual `workflow_dispatch`), the deploy falls back to deploying. -## `ci.yml` — Continuous Integration +## `ci.yml` — CI A single `prepare` job detects everything once and exposes it as outputs; the heavy jobs fan out from it and are **gated by change detection** so a package-only PR never @@ -184,7 +190,7 @@ Codecov merges uploads by commit SHA. 100% coverage is enforced inside each | `storybook` | Storybook test runner | | `e2e` | web Playwright (`coverage/e2e`) | -## `cd-web.yml` — GitHub Pages + Storybook deploy +## `cd-web.yml` — CD · Web (Pages + Storybook) ```mermaid flowchart TD @@ -201,7 +207,7 @@ flowchart TD abort each other. Build env (Vite vars, GitHub key, Turnstile sitekey) is injected from repo secrets/vars; `APP_ENV=production`. -## `cd-worker-api.yml` — Cloudflare Worker deploy +## `cd-worker-api.yml` — CD · Worker (api) ```mermaid flowchart TD @@ -214,13 +220,13 @@ flowchart TD `config:gen` renders `wrangler.json` from repo `vars` (worker name, D1, R2, honeypot); `wrangler deploy` authenticates with `CLOUDFLARE_API_TOKEN` + `CLOUDFLARE_ACCOUNT_ID`. -## `cd-worker-bench.yml` — Bench relay Worker deploy +## `cd-worker-bench.yml` — CD · Worker (bench) Structural mirror of `cd-worker-api.yml` for `workers/bench` (the bench-action comment relay at `api.bench.soroush.tech`): deploys when `worker∋bench ∥ root`, in its own `cd-worker-bench` environment — see [`cd-worker-bench.md`](./cd-worker-bench.md). -## `cd-packages.yml` — npm publish +## `cd-packages.yml` — CD · Packages (npm) **Manual only** — unlike the other two CD workflows, this one is **not** gated on CI and never runs off a push, PR merge, or `workflow_run`. It publishes from `workflow_dispatch`, diff --git a/.github/workflows/cd-editor.md b/.github/workflows/cd-editor.md index e1b67870..623a8101 100644 --- a/.github/workflows/cd-editor.md +++ b/.github/workflows/cd-editor.md @@ -1,6 +1,6 @@ [← Workflows overview](./README.md) -# `cd-editor.yml` — Package and release the desktop editor +# `cd-editor.yml` — CD · Editor (release) Builds the editor's installers on both platforms and assembles them into **one published GitHub Release** with a title and the notes from diff --git a/.github/workflows/cd-editor.yml b/.github/workflows/cd-editor.yml index 8b045eb6..1d8736d0 100644 --- a/.github/workflows/cd-editor.yml +++ b/.github/workflows/cd-editor.yml @@ -1,6 +1,6 @@ # ci:validates nothing # Packages the desktop editor and publishes one GitHub Release. -name: CD · Editor +name: CD · Editor (release) # Manual only — a release is a decision, not a side effect of a merge, and # approving the dispatch is the release act. The two build legs upload their diff --git a/.github/workflows/cd-packages.md b/.github/workflows/cd-packages.md index d940d6c3..f8073607 100644 --- a/.github/workflows/cd-packages.md +++ b/.github/workflows/cd-packages.md @@ -1,6 +1,6 @@ [← Workflows overview](./README.md) -# `cd-packages.yml` — Publish Packages (npm) +# `cd-packages.yml` — CD · Packages (npm) Publishes `@soroush.tech/*` packages to npm via **Trusted Publishing (OIDC)** — no long-lived `NPM_TOKEN`. The dispatch picks either a single package or **`all`**: every @@ -153,7 +153,7 @@ catch drift that publishing can't act on. 1. In one PR to `main` (CI runs): bump the package `version` in `package.json` **and** add `packages//release-notes/.md` with that version's notes — for as many packages as the PR releases. -2. Actions → **Publish Packages (npm)** → **Run workflow** → pick the `package` (or `all` +2. Actions → **CD · Packages (npm)** → **Run workflow** → pick the `package` (or `all` to release every package with a pending bump), **Run**. CLI equivalent: `gh workflow run cd-packages.yml -f package=`. 3. The job publishes each resolved package to npm (skipping any version already there) and diff --git a/.github/workflows/cd-packages.yml b/.github/workflows/cd-packages.yml index 003ee1ed..f58da370 100644 --- a/.github/workflows/cd-packages.yml +++ b/.github/workflows/cd-packages.yml @@ -1,6 +1,6 @@ # ci:validates nothing # CI never runs this workflow, so no job here could prove anything about it. -name: Publish Packages (npm) +name: CD · Packages (npm) # Publishes non-private packages to npm via Trusted Publishing (OIDC): a single named # package, or `all` — every non-private package whose current version isn't on the diff --git a/.github/workflows/cd-web.md b/.github/workflows/cd-web.md index 3b8416de..ae5c1926 100644 --- a/.github/workflows/cd-web.md +++ b/.github/workflows/cd-web.md @@ -1,17 +1,17 @@ [← Workflows overview](./README.md) -# `cd-web.yml` — Continuous Deployment to GitHub Pages +# `cd-web.yml` — CD · Web (Pages + Storybook) Builds the web app and deploys it to GitHub Pages, and in parallel builds Storybook and deploys it to a Cloudflare Pages site at [storybook.soroush.tech](https://storybook.soroush.tech). **Gated on CI success** — it -starts from a `workflow_run` of `Continuous Integration`, never from a raw `push`. +starts from a `workflow_run` of `CI`, never from a raw `push`. ```yaml on: workflow_dispatch: workflow_run: - workflows: ['Continuous Integration'] + workflows: ['CI'] types: [completed] branches: [main] permissions: { contents: read, pages: write, id-token: write } diff --git a/.github/workflows/cd-web.yml b/.github/workflows/cd-web.yml index f5441878..12727e41 100644 --- a/.github/workflows/cd-web.yml +++ b/.github/workflows/cd-web.yml @@ -1,10 +1,10 @@ # ci:validates nothing # CI never runs this workflow, so no job here could prove anything about it. -name: Continuous Deployment to GitHub Pages +name: CD · Web (Pages + Storybook) on: workflow_dispatch: workflow_run: - workflows: ['Continuous Integration'] + workflows: ['CI'] types: - completed branches: diff --git a/.github/workflows/cd-worker-api.md b/.github/workflows/cd-worker-api.md index 93e0978b..49cc725b 100644 --- a/.github/workflows/cd-worker-api.md +++ b/.github/workflows/cd-worker-api.md @@ -1,15 +1,15 @@ [← Workflows overview](./README.md) -# `cd-worker-api.yml` — Deploy Worker (`@soroush/api`) +# `cd-worker-api.yml` — CD · Worker (api) Deploys the Cloudflare Worker API. **Gated on CI success** — starts from a -`workflow_run` of `Continuous Integration`, never from a raw `push`. +`workflow_run` of `CI`, never from a raw `push`. ```yaml on: workflow_dispatch: workflow_run: - workflows: ['Continuous Integration'] + workflows: ['CI'] types: [completed] branches: [main] concurrency: diff --git a/.github/workflows/cd-worker-api.yml b/.github/workflows/cd-worker-api.yml index 4865fb58..8e755009 100644 --- a/.github/workflows/cd-worker-api.yml +++ b/.github/workflows/cd-worker-api.yml @@ -1,11 +1,11 @@ # ci:validates nothing # CI never runs this workflow, so no job here could prove anything about it. -name: Deploy Worker (@soroush/api) +name: CD · Worker (api) on: workflow_dispatch: workflow_run: - workflows: ['Continuous Integration'] + workflows: ['CI'] types: - completed branches: diff --git a/.github/workflows/cd-worker-bench.md b/.github/workflows/cd-worker-bench.md index 871589a9..afb16bf8 100644 --- a/.github/workflows/cd-worker-bench.md +++ b/.github/workflows/cd-worker-bench.md @@ -1,11 +1,11 @@ [← Workflows overview](./README.md) -# `cd-worker-bench.yml` — Deploy Worker (`@soroush/bench-api`) +# `cd-worker-bench.yml` — CD · Worker (bench) Deploys the bench-action comment relay (`workers/bench`, served at `api.bench.soroush.tech` — see [`workers/bench/worker.md`](../../workers/bench/worker.md)). Structurally a mirror of [`cd-worker-api.md`](./cd-worker-api.md): **gated on CI success** -(`workflow_run` of `Continuous Integration` + manual `workflow_dispatch`), concurrency group +(`workflow_run` of `CI` + manual `workflow_dispatch`), concurrency group `deploy-worker-bench` with `cancel-in-progress: false`. ## Job: `changes` diff --git a/.github/workflows/cd-worker-bench.yml b/.github/workflows/cd-worker-bench.yml index b7eaaf28..3c654743 100644 --- a/.github/workflows/cd-worker-bench.yml +++ b/.github/workflows/cd-worker-bench.yml @@ -1,11 +1,11 @@ # ci:validates nothing # CI never runs this workflow, so no job here could prove anything about it. -name: Deploy Worker (@soroush/bench-api) +name: CD · Worker (bench) on: workflow_dispatch: workflow_run: - workflows: ['Continuous Integration'] + workflows: ['CI'] types: - completed branches: diff --git a/.github/workflows/ci.md b/.github/workflows/ci.md index f504f957..fe211c86 100644 --- a/.github/workflows/ci.md +++ b/.github/workflows/ci.md @@ -1,6 +1,6 @@ [← Workflows overview](./README.md) -# `ci.yml` — Continuous Integration +# `ci.yml` — CI Entry workflow for the monorepo. A `prepare` job detects everything once, `lint` covers the workspace, and each area's jobs run through a called workflow of its own. `ci-ok` is the single diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a69fd4d4..f4eb56ae 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,6 +1,6 @@ # ci:validates all # ci.yml holds prepare, lint and ci-ok — it decides how every job runs, so every job proves it. -name: Continuous Integration +name: CI on: push: From 897b932ff4fee72f53225053e394cae55a088d5f Mon Sep 17 00:00:00 2001 From: Masoud Soroush Date: Sun, 9 Aug 2026 20:57:01 +0200 Subject: [PATCH 3/6] chore(CI): unnest the reason template and complete the bench deploy condition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings. `attributeWorkflows` built its reason strings from a template literal nested inside another one, which is hard to read at the point where it matters least. The path and the meaning are now named, and the reason is one flat template. Output is byte-identical. The workflows README summarised the bench deploy as `worker∋bench ∥ root`, but cd-worker-bench.yml has gated on `packages∋wrangler-tools` since the deploy started rendering wrangler.json with the shared bin — as cd-worker-bench.md already documented. The overview was the only place saying otherwise. --- .github/workflows/README.md | 3 ++- scripts/assemble-changes.mjs | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 97cfd558..51150f2e 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -223,7 +223,8 @@ flowchart TD ## `cd-worker-bench.yml` — CD · Worker (bench) Structural mirror of `cd-worker-api.yml` for `workers/bench` (the bench-action comment -relay at `api.bench.soroush.tech`): deploys when `worker∋bench ∥ root`, in its own +relay at `api.bench.soroush.tech`): deploys when +`worker∋bench ∥ packages∋wrangler-tools ∥ root`, in its own `cd-worker-bench` environment — see [`cd-worker-bench.md`](./cd-worker-bench.md). ## `cd-packages.yml` — CD · Packages (npm) diff --git a/scripts/assemble-changes.mjs b/scripts/assemble-changes.mjs index 82e7f0c7..bbb8d354 100644 --- a/scripts/assemble-changes.mjs +++ b/scripts/assemble-changes.mjs @@ -219,14 +219,16 @@ export function workflowValidates(name) { export function attributeWorkflows(workflows) { const files = ciFiles() const claims = workflows.map((name) => ({ name, ...workflowValidates(name) })) + // By path rather than by name: not every one of these is `.yml` any more. + const pathOf = (name) => files[name] ?? `${name}.yml` + const meaning = ({ keys, wholeWorkspace }) => { + if (wholeWorkspace) return 'it decides how every job runs' + return keys.join(', ') || 'CI never runs it' + } return { keys: claims.flatMap(({ keys }) => keys), wholeWorkspace: claims.some((claim) => claim.wholeWorkspace), - // By path rather than by name: not every one of these is `.yml` any more. - reasons: claims.map( - ({ name, keys, wholeWorkspace }) => - `${files[name] ?? `${name}.yml`} changed — ${wholeWorkspace ? 'it decides how every job runs' : keys.join(', ') || 'CI never runs it'}` - ), + reasons: claims.map((claim) => `${pathOf(claim.name)} changed — ${meaning(claim)}`), } } From 8ff77a079d6adefaf9a056bd346bb11ce881af5b Mon Sep 17 00:00:00 2001 From: Masoud Soroush Date: Sun, 9 Aug 2026 21:56:47 +0200 Subject: [PATCH 4/6] fix(CI): validate everything when a CI file is deleted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-file `wf__` keys are built by reading the working tree, so a deleted workflow or action generates no key: the one thing that changed matched nothing. A pull request that only removes CI files therefore ran no job at all and reported green — the same silent under-run the marker parser already refuses to make when it cannot read a claim. `ci__any` covers `.github/workflows/**` and `.github/actions/**`. When it matches and no per-file key does, the change is a file that is no longer there, and the whole workspace is validated. `changes.root` stays false, so a deletion still ships nothing. Coarse on purpose: it knows a file went, not which one, so it says "all" rather than guessing. A deletion alongside an edit needs no help, since the edit makes its own claim. Deleting only a workflow now runs both apps, all twelve packages and both workers, with root false. Editing one still validates just that file's areas, and a `cd-*` edit still validates nothing. --- .github/workflows/ci.md | 29 ++++++++++++++++++----------- scripts/assemble-changes.mjs | 31 ++++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.md b/.github/workflows/ci.md index fe211c86..f6d7ab21 100644 --- a/.github/workflows/ci.md +++ b/.github/workflows/ci.md @@ -58,17 +58,17 @@ and a one-line change to how the editor runs its tests re-runs the editor. See `runs-on: ubuntu-latest` · `timeout-minutes: 15`. Produces every output the other jobs consume via `needs.prepare.outputs.*`. -| # | Step | Run / Action | What it does | -| --- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | Checkout Repository | `actions/checkout@v5` (`persist-credentials: false`) | Clone the repo without leaving the token on disk. | -| 2 | Read Node.js version | `cat .nvmrc` → `$GITHUB_OUTPUT` | Single source of truth for the Node version; never hard-coded. | -| 3 | Detect package manager | shell `if` on lockfile presence | Emits `manager` (`pnpm`/`yarn`/`npm`), `command` (e.g. `install --frozen-lockfile --ignore-scripts` — no lifecycle script runs at install anywhere in CI), `runner`. Fails if none found. | -| 4 | Read Playwright version | `node -p "...devDependencies?.['@playwright/test'] \|\| ...dependencies?.['@playwright/test']"` then strip leading `^` | Feeds the Playwright binary cache key. **Must read `@playwright/test`** — the project has no bare `playwright` dep, so reading `playwright` yields the string `"undefined"` and freezes the cache key (see [Caching](#caching)). | -| 5 | Discover workspace entities | `node scripts/assemble-changes.mjs filters` | Builds a per-entity `paths-filter` config: one key per app (`app__`), worker (`worker__`), package (`pkg__`), CI definition file (`wf__` — every workflow, plus `wf__actions-setup` for the [setup action](#the-setup-action)), and **one per root file** (`root__lock`, `root__workspace`, `root__package`, `root__nvmrc`, `root__tsconfig`) — a whitelist, so root docs/tooling dotfiles trigger nothing. | -| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched. | -| 7 | Copy base manifest + lockfile | `git show "$BASE_SHA:…"` into `$RUNNER_TEMP/base`, fetching one commit deep first if the shallow clone lacks it | Hands the next step the two files it compares against. Every command may fail without failing the job — a missing or empty copy reads as "cannot be compared", which validates everything. | -| 8 | Assemble `changes.json` | `node scripts/assemble-changes.mjs assemble` | Writes [`changes.json`](#changesjson) (the lists + `root`), and derives this run's own gating outputs `web` / `editor` / `has_packages` / `changed_packages` / `has_workers` / `changed_workers`. Root files are attributed first — see [What a root change means](#what-a-root-change-means). A whole-workspace root file, or a change to `ci.yml` itself, means every job runs. | -| 9 | Upload `changes.json` | `actions/upload-artifact@v7` (name `changes`) | Hands the single file to the CD workflows, which run on `workflow_run` and have no diff base of their own. | +| # | Step | Run / Action | What it does | +| --- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | Checkout Repository | `actions/checkout@v5` (`persist-credentials: false`) | Clone the repo without leaving the token on disk. | +| 2 | Read Node.js version | `cat .nvmrc` → `$GITHUB_OUTPUT` | Single source of truth for the Node version; never hard-coded. | +| 3 | Detect package manager | shell `if` on lockfile presence | Emits `manager` (`pnpm`/`yarn`/`npm`), `command` (e.g. `install --frozen-lockfile --ignore-scripts` — no lifecycle script runs at install anywhere in CI), `runner`. Fails if none found. | +| 4 | Read Playwright version | `node -p "...devDependencies?.['@playwright/test'] \|\| ...dependencies?.['@playwright/test']"` then strip leading `^` | Feeds the Playwright binary cache key. **Must read `@playwright/test`** — the project has no bare `playwright` dep, so reading `playwright` yields the string `"undefined"` and freezes the cache key (see [Caching](#caching)). | +| 5 | Discover workspace entities | `node scripts/assemble-changes.mjs filters` | Builds a per-entity `paths-filter` config: one key per app (`app__`), worker (`worker__`), package (`pkg__`), CI definition file (`wf__` — every workflow, plus `wf__actions-setup` for the [setup action](#the-setup-action)), and **one per root file** (`root__lock`, `root__workspace`, `root__package`, `root__nvmrc`, `root__tsconfig`) plus `ci__any` over every CI path (see below) — a whitelist, so root docs/tooling dotfiles trigger nothing. | +| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched. | +| 7 | Copy base manifest + lockfile | `git show "$BASE_SHA:…"` into `$RUNNER_TEMP/base`, fetching one commit deep first if the shallow clone lacks it | Hands the next step the two files it compares against. Every command may fail without failing the job — a missing or empty copy reads as "cannot be compared", which validates everything. | +| 8 | Assemble `changes.json` | `node scripts/assemble-changes.mjs assemble` | Writes [`changes.json`](#changesjson) (the lists + `root`), and derives this run's own gating outputs `web` / `editor` / `has_packages` / `changed_packages` / `has_workers` / `changed_workers`. Root files are attributed first — see [What a root change means](#what-a-root-change-means). A whole-workspace root file, or a change to `ci.yml` itself, means every job runs. | +| 9 | Upload `changes.json` | `actions/upload-artifact@v7` (name `changes`) | Hands the single file to the CD workflows, which run on `workflow_run` and have no diff base of their own. | ### Outputs @@ -245,6 +245,13 @@ unreadable claim must over-run, never quietly under-run. The marker line holds t goes on the line below, because a trailing "…and nothing else" is enough to make a file claim it validates nothing. +A **deleted** file has no marker left to read, and no key either: the per-file keys are built from +the working tree, so the one thing that changed generates nothing to match. That is what the +`ci__any` catch-all is for — it covers `.github/workflows/**` and `.github/actions/**`, so a CI +change that no per-file key accounts for still validates the whole workspace instead of passing as +"nothing changed". Coarse on purpose: it knows a file went, not which one. A deletion alongside an +edit needs no help, since the edit makes its own claim. + This attribution gates jobs and **never reaches `changes.json`**. Editing `ci-web.yml` must re-run the web suite without shipping the site — and `cd-web.yml` deploys on `changes.apps` containing `web`. diff --git a/scripts/assemble-changes.mjs b/scripts/assemble-changes.mjs index bbb8d354..827f6233 100644 --- a/scripts/assemble-changes.mjs +++ b/scripts/assemble-changes.mjs @@ -65,9 +65,16 @@ export function ciFiles() { return files } +/** + * One key over every CI definition path, matched against the per-file keys to catch a file that + * `ciFiles()` cannot see — see `hasUnattributedCiFile`. + */ +const CI_ANY = 'ci__any' + /** * The paths-filter config: one key per workspace member, per CI definition file, and per root - * file. JSON is valid YAML, so the action takes this as its `filters` input verbatim. + * file, plus the catch-all. JSON is valid YAML, so the action takes this as its `filters` input + * verbatim. */ export function buildFilters() { const filters = {} @@ -75,10 +82,25 @@ export function buildFilters() { for (const name of dirsWithPackageJson(dir)) filters[`${prefix}${name}`] = [`${dir}/${name}/**`] } for (const [name, file] of Object.entries(ciFiles())) filters[`wf__${name}`] = [file] + filters[CI_ANY] = ['.github/workflows/**', '.github/actions/**'] for (const [key, file] of Object.entries(ROOT_FILES)) filters[key] = [file] return filters } +/** + * True when a CI definition file changed that no `wf__` key claimed. `ciFiles()` reads the working + * tree, so a **deleted** workflow or action generates no key of its own: without this, a change + * that removes one is a change nothing matched, and a pull request deleting CI files alone runs no + * job at all and reports green. The catch-all sees the path either way, so a CI change no per-file + * key accounts for validates everything — the same answer an unreadable marker gets, for the same + * reason: this must over-run, never quietly under-run. + * + * Deliberately coarse. It cannot tell which file went, only that one did, so it says "all" rather + * than guessing. A deletion alongside an edit is already covered by the edit's own claim. + */ +export const hasUnattributedCiFile = (changed) => + changed.includes(CI_ANY) && !changed.some((key) => key.startsWith('wf__')) + /** * The importer blocks of a pnpm lockfile, keyed by workspace path. Sectioned by indentation * rather than parsed: prepare runs before any install, so there is no YAML parser to reach for, @@ -433,14 +455,17 @@ function main() { const workflows = attributeWorkflows( changed.filter((key) => key.startsWith('wf__')).map((key) => key.slice('wf__'.length)) ) - for (const reason of [...root.reasons, ...workflows.reasons]) console.error(`changes: ${reason}`) + const removedCiFile = hasUnattributedCiFile(changed) + const reasons = [...root.reasons, ...workflows.reasons] + if (removedCiFile) reasons.push('a CI file changed that no per-file key claimed — it is gone') + for (const reason of reasons) console.error(`changes: ${reason}`) const { changes, outputs } = assembleChanges({ changed, members, attributed: root.attributed, revalidate: workflows.keys, - revalidateAll: workflows.wholeWorkspace, + revalidateAll: workflows.wholeWorkspace || removedCiFile, wholeWorkspace: root.wholeWorkspace, }) From 2659e6b407d475d27bce7307150533f7859d0456 Mon Sep 17 00:00:00 2001 From: Masoud Soroush Date: Tue, 11 Aug 2026 17:13:44 +0200 Subject: [PATCH 5/6] fix(CI): match CI file deletions with a status predicate, not a key heuristic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hasUnattributedCiFile inferred a deletion from "ci__any matched and no wf__ key did" — but a deletion beside an ordinary workflow edit produces both keys, so the full-workspace fallback stayed off while the edit's own claim can be far narrower than what the deleted file validated. The new ci__deleted filter uses paths-filter's `deleted:` change-type predicate over the same two globs, so it fires on the deletion itself no matter what changed alongside it; the key heuristic stays for a changed CI file that exists but carries no key. --- .github/workflows/ci.md | 32 +++++++++++++++++--------------- scripts/assemble-changes.mjs | 36 ++++++++++++++++++++++++------------ 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.md b/.github/workflows/ci.md index f6d7ab21..73feb990 100644 --- a/.github/workflows/ci.md +++ b/.github/workflows/ci.md @@ -58,17 +58,17 @@ and a one-line change to how the editor runs its tests re-runs the editor. See `runs-on: ubuntu-latest` · `timeout-minutes: 15`. Produces every output the other jobs consume via `needs.prepare.outputs.*`. -| # | Step | Run / Action | What it does | -| --- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| 1 | Checkout Repository | `actions/checkout@v5` (`persist-credentials: false`) | Clone the repo without leaving the token on disk. | -| 2 | Read Node.js version | `cat .nvmrc` → `$GITHUB_OUTPUT` | Single source of truth for the Node version; never hard-coded. | -| 3 | Detect package manager | shell `if` on lockfile presence | Emits `manager` (`pnpm`/`yarn`/`npm`), `command` (e.g. `install --frozen-lockfile --ignore-scripts` — no lifecycle script runs at install anywhere in CI), `runner`. Fails if none found. | -| 4 | Read Playwright version | `node -p "...devDependencies?.['@playwright/test'] \|\| ...dependencies?.['@playwright/test']"` then strip leading `^` | Feeds the Playwright binary cache key. **Must read `@playwright/test`** — the project has no bare `playwright` dep, so reading `playwright` yields the string `"undefined"` and freezes the cache key (see [Caching](#caching)). | -| 5 | Discover workspace entities | `node scripts/assemble-changes.mjs filters` | Builds a per-entity `paths-filter` config: one key per app (`app__`), worker (`worker__`), package (`pkg__`), CI definition file (`wf__` — every workflow, plus `wf__actions-setup` for the [setup action](#the-setup-action)), and **one per root file** (`root__lock`, `root__workspace`, `root__package`, `root__nvmrc`, `root__tsconfig`) plus `ci__any` over every CI path (see below) — a whitelist, so root docs/tooling dotfiles trigger nothing. | -| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched. | -| 7 | Copy base manifest + lockfile | `git show "$BASE_SHA:…"` into `$RUNNER_TEMP/base`, fetching one commit deep first if the shallow clone lacks it | Hands the next step the two files it compares against. Every command may fail without failing the job — a missing or empty copy reads as "cannot be compared", which validates everything. | -| 8 | Assemble `changes.json` | `node scripts/assemble-changes.mjs assemble` | Writes [`changes.json`](#changesjson) (the lists + `root`), and derives this run's own gating outputs `web` / `editor` / `has_packages` / `changed_packages` / `has_workers` / `changed_workers`. Root files are attributed first — see [What a root change means](#what-a-root-change-means). A whole-workspace root file, or a change to `ci.yml` itself, means every job runs. | -| 9 | Upload `changes.json` | `actions/upload-artifact@v7` (name `changes`) | Hands the single file to the CD workflows, which run on `workflow_run` and have no diff base of their own. | +| # | Step | Run / Action | What it does | +| --- | ----------------------------- | ---------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| 1 | Checkout Repository | `actions/checkout@v5` (`persist-credentials: false`) | Clone the repo without leaving the token on disk. | +| 2 | Read Node.js version | `cat .nvmrc` → `$GITHUB_OUTPUT` | Single source of truth for the Node version; never hard-coded. | +| 3 | Detect package manager | shell `if` on lockfile presence | Emits `manager` (`pnpm`/`yarn`/`npm`), `command` (e.g. `install --frozen-lockfile --ignore-scripts` — no lifecycle script runs at install anywhere in CI), `runner`. Fails if none found. | +| 4 | Read Playwright version | `node -p "...devDependencies?.['@playwright/test'] \|\| ...dependencies?.['@playwright/test']"` then strip leading `^` | Feeds the Playwright binary cache key. **Must read `@playwright/test`** — the project has no bare `playwright` dep, so reading `playwright` yields the string `"undefined"` and freezes the cache key (see [Caching](#caching)). | +| 5 | Discover workspace entities | `node scripts/assemble-changes.mjs filters` | Builds a per-entity `paths-filter` config: one key per app (`app__`), worker (`worker__`), package (`pkg__`), CI definition file (`wf__` — every workflow, plus `wf__actions-setup` for the [setup action](#the-setup-action)), and **one per root file** (`root__lock`, `root__workspace`, `root__package`, `root__nvmrc`, `root__tsconfig`) plus `ci__any` over every CI path and `ci__deleted` matching only CI-path deletions (see below) — a whitelist, so root docs/tooling dotfiles trigger nothing. | +| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched. | +| 7 | Copy base manifest + lockfile | `git show "$BASE_SHA:…"` into `$RUNNER_TEMP/base`, fetching one commit deep first if the shallow clone lacks it | Hands the next step the two files it compares against. Every command may fail without failing the job — a missing or empty copy reads as "cannot be compared", which validates everything. | +| 8 | Assemble `changes.json` | `node scripts/assemble-changes.mjs assemble` | Writes [`changes.json`](#changesjson) (the lists + `root`), and derives this run's own gating outputs `web` / `editor` / `has_packages` / `changed_packages` / `has_workers` / `changed_workers`. Root files are attributed first — see [What a root change means](#what-a-root-change-means). A whole-workspace root file, or a change to `ci.yml` itself, means every job runs. | +| 9 | Upload `changes.json` | `actions/upload-artifact@v7` (name `changes`) | Hands the single file to the CD workflows, which run on `workflow_run` and have no diff base of their own. | ### Outputs @@ -247,10 +247,12 @@ validates nothing. A **deleted** file has no marker left to read, and no key either: the per-file keys are built from the working tree, so the one thing that changed generates nothing to match. That is what the -`ci__any` catch-all is for — it covers `.github/workflows/**` and `.github/actions/**`, so a CI -change that no per-file key accounts for still validates the whole workspace instead of passing as -"nothing changed". Coarse on purpose: it knows a file went, not which one. A deletion alongside an -edit needs no help, since the edit makes its own claim. +`ci__deleted` filter is for — a `deleted:` status predicate over `.github/workflows/**` and +`.github/actions/**`, so it fires on the deletion itself, even beside an edit whose own claim +would otherwise mask it (from matched keys alone, deletion-plus-edit looks exactly like the edit). +The `ci__any` catch-all backs it up for a CI file that still exists but carries no key. Either way +the whole workspace validates instead of passing as "nothing changed" — coarse on purpose: it +knows a file went, not which one. This attribution gates jobs and **never reaches `changes.json`**. Editing `ci-web.yml` must re-run the web suite without shipping the site — and `cd-web.yml` deploys on `changes.apps` containing diff --git a/scripts/assemble-changes.mjs b/scripts/assemble-changes.mjs index 827f6233..e002c703 100644 --- a/scripts/assemble-changes.mjs +++ b/scripts/assemble-changes.mjs @@ -71,10 +71,18 @@ export function ciFiles() { */ const CI_ANY = 'ci__any' +/** + * Matches only when a CI definition file was **deleted** — a paths-filter status predicate over + * the same two globs as `CI_ANY`, so it fires on the deletion itself, no matter what else changed + * alongside it. This is the signal no key heuristic can reconstruct: matched keys cannot tell a + * deletion beside an edit from the edit alone. + */ +const CI_DELETED = 'ci__deleted' + /** * The paths-filter config: one key per workspace member, per CI definition file, and per root - * file, plus the catch-all. JSON is valid YAML, so the action takes this as its `filters` input - * verbatim. + * file, plus the two catch-alls. JSON is valid YAML, so the action takes this as its `filters` + * input verbatim. */ export function buildFilters() { const filters = {} @@ -83,20 +91,23 @@ export function buildFilters() { } for (const [name, file] of Object.entries(ciFiles())) filters[`wf__${name}`] = [file] filters[CI_ANY] = ['.github/workflows/**', '.github/actions/**'] + filters[CI_DELETED] = [{ deleted: '.github/workflows/**' }, { deleted: '.github/actions/**' }] for (const [key, file] of Object.entries(ROOT_FILES)) filters[key] = [file] return filters } /** - * True when a CI definition file changed that no `wf__` key claimed. `ciFiles()` reads the working - * tree, so a **deleted** workflow or action generates no key of its own: without this, a change - * that removes one is a change nothing matched, and a pull request deleting CI files alone runs no - * job at all and reports green. The catch-all sees the path either way, so a CI change no per-file - * key accounts for validates everything — the same answer an unreadable marker gets, for the same - * reason: this must over-run, never quietly under-run. + * True when a CI definition file changed that no `wf__` key claimed. The per-file keys only cover + * what `ciFiles()` names, so a CI file outside that set matches the catch-all and nothing else: + * without this, a pull request changing only such files runs no job at all and reports green. It + * validates everything — the same answer an unreadable marker gets, for the same reason: this + * must over-run, never quietly under-run. * - * Deliberately coarse. It cannot tell which file went, only that one did, so it says "all" rather - * than guessing. A deletion alongside an edit is already covered by the edit's own claim. + * Blind past the first claim, though: from matched keys alone, a deletion beside an edit looks + * exactly like the edit by itself, and the edit's claim can be far narrower than what the deleted + * file validated. Deletions are therefore caught by `CI_DELETED` — a status predicate, not a key + * heuristic — and this covers only what that cannot: a changed file that still exists but carries + * no key. */ export const hasUnattributedCiFile = (changed) => changed.includes(CI_ANY) && !changed.some((key) => key.startsWith('wf__')) @@ -455,9 +466,10 @@ function main() { const workflows = attributeWorkflows( changed.filter((key) => key.startsWith('wf__')).map((key) => key.slice('wf__'.length)) ) - const removedCiFile = hasUnattributedCiFile(changed) + const removedCiFile = changed.includes(CI_DELETED) || hasUnattributedCiFile(changed) const reasons = [...root.reasons, ...workflows.reasons] - if (removedCiFile) reasons.push('a CI file changed that no per-file key claimed — it is gone') + if (changed.includes(CI_DELETED)) reasons.push('a CI file was deleted — nothing left to claim it') + else if (removedCiFile) reasons.push('a CI file changed that no per-file key claimed') for (const reason of reasons) console.error(`changes: ${reason}`) const { changes, outputs } = assembleChanges({ From b80313a41fadb43a3f6cfdcd2aa2e3495375a490 Mon Sep 17 00:00:00 2001 From: Masoud Soroush Date: Wed, 12 Aug 2026 00:20:15 +0200 Subject: [PATCH 6/6] fix(CI): attribute unclaimed CI files by path, and let docs rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The key heuristic went blind past the first claim: a CI file no per-file key names — an aux file inside an action, or a deleted workflow with the predicate filter gone — changed beside an ordinary workflow edit produced the same matched keys as the edit alone, and the edit's claim can be far narrower than what the masked file validated. paths-filter now hands assemble the paths ci__any matched (list-files: json); anything outside ciFiles() triggers full validation and is named in the log, a list that fails to arrive counts as everything, and ci__deleted stays as the deletion backstop. The .md companions are exempt both solo and mixed: they are documentation no job reads, and a README typo ran every job. --- .github/workflows/ci.md | 19 ++++++------ .github/workflows/ci.yml | 3 ++ scripts/assemble-changes.mjs | 60 ++++++++++++++++++++++++------------ 3 files changed, 53 insertions(+), 29 deletions(-) diff --git a/.github/workflows/ci.md b/.github/workflows/ci.md index 73feb990..0a23a4c8 100644 --- a/.github/workflows/ci.md +++ b/.github/workflows/ci.md @@ -65,7 +65,7 @@ jobs consume via `needs.prepare.outputs.*`. | 3 | Detect package manager | shell `if` on lockfile presence | Emits `manager` (`pnpm`/`yarn`/`npm`), `command` (e.g. `install --frozen-lockfile --ignore-scripts` — no lifecycle script runs at install anywhere in CI), `runner`. Fails if none found. | | 4 | Read Playwright version | `node -p "...devDependencies?.['@playwright/test'] \|\| ...dependencies?.['@playwright/test']"` then strip leading `^` | Feeds the Playwright binary cache key. **Must read `@playwright/test`** — the project has no bare `playwright` dep, so reading `playwright` yields the string `"undefined"` and freezes the cache key (see [Caching](#caching)). | | 5 | Discover workspace entities | `node scripts/assemble-changes.mjs filters` | Builds a per-entity `paths-filter` config: one key per app (`app__`), worker (`worker__`), package (`pkg__`), CI definition file (`wf__` — every workflow, plus `wf__actions-setup` for the [setup action](#the-setup-action)), and **one per root file** (`root__lock`, `root__workspace`, `root__package`, `root__nvmrc`, `root__tsconfig`) plus `ci__any` over every CI path and `ci__deleted` matching only CI-path deletions (see below) — a whitelist, so root docs/tooling dotfiles trigger nothing. | -| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched. | +| 6 | Detect changed entities | `dorny/paths-filter@v4` | Consumes the generated `filters` (JSON is valid YAML) and outputs a `changes` list of the keys that matched, plus — via `list-files: json` — the matched paths per key; assemble reads `ci__any`'s list. | | 7 | Copy base manifest + lockfile | `git show "$BASE_SHA:…"` into `$RUNNER_TEMP/base`, fetching one commit deep first if the shallow clone lacks it | Hands the next step the two files it compares against. Every command may fail without failing the job — a missing or empty copy reads as "cannot be compared", which validates everything. | | 8 | Assemble `changes.json` | `node scripts/assemble-changes.mjs assemble` | Writes [`changes.json`](#changesjson) (the lists + `root`), and derives this run's own gating outputs `web` / `editor` / `has_packages` / `changed_packages` / `has_workers` / `changed_workers`. Root files are attributed first — see [What a root change means](#what-a-root-change-means). A whole-workspace root file, or a change to `ci.yml` itself, means every job runs. | | 9 | Upload `changes.json` | `actions/upload-artifact@v7` (name `changes`) | Hands the single file to the CD workflows, which run on `workflow_run` and have no diff base of their own. | @@ -245,14 +245,15 @@ unreadable claim must over-run, never quietly under-run. The marker line holds t goes on the line below, because a trailing "…and nothing else" is enough to make a file claim it validates nothing. -A **deleted** file has no marker left to read, and no key either: the per-file keys are built from -the working tree, so the one thing that changed generates nothing to match. That is what the -`ci__deleted` filter is for — a `deleted:` status predicate over `.github/workflows/**` and -`.github/actions/**`, so it fires on the deletion itself, even beside an edit whose own claim -would otherwise mask it (from matched keys alone, deletion-plus-edit looks exactly like the edit). -The `ci__any` catch-all backs it up for a CI file that still exists but carries no key. Either way -the whole workspace validates instead of passing as "nothing changed" — coarse on purpose: it -knows a file went, not which one. +A CI file that carries no key of its own — a **deleted** workflow (the keys are built from the +working tree, so the one thing that changed generates nothing to match), or an aux file no filter +names — cannot make a claim. So `prepare` compares paths, not keys: paths-filter lists every file +`ci__any` matched (`list-files: json`), and assemble subtracts the keyed files and the `.md` +companions, which are documentation no job reads. Anything left validates the whole workspace +instead of passing as "nothing changed" — even beside an edit whose own claim would otherwise +mask it, since from matched keys alone that mix looks exactly like the edit by itself. A file +list that fails to arrive counts as "everything" too, and the `ci__deleted` status predicate +(`deleted:` over the same two globs) backs the deletion case up independently of the list. This attribution gates jobs and **never reaches `changes.json`**. Editing `ci-web.yml` must re-run the web suite without shipping the site — and `cd-web.yml` deploys on `changes.apps` containing diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f4eb56ae..c118ed54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -87,6 +87,8 @@ jobs: id: filter with: filters: ${{ steps.discover.outputs.filters }} # JSON is valid YAML + # Assemble reads ci__any's matched paths to spot CI files no per-file key claims. + list-files: json # The root manifest and the lockfile are attributed by comparing them against the # base — the target branch for a pull request, the previous tip for a push — so both @@ -111,6 +113,7 @@ jobs: id: assemble env: CHANGES: ${{ steps.filter.outputs.changes }} + CI_ANY_FILES: ${{ steps.filter.outputs.ci__any_files }} BASE_PACKAGE_JSON: ${{ runner.temp }}/base/package.json BASE_LOCKFILE: ${{ runner.temp }}/base/pnpm-lock.yaml run: node scripts/assemble-changes.mjs assemble >> "$GITHUB_OUTPUT" diff --git a/scripts/assemble-changes.mjs b/scripts/assemble-changes.mjs index e002c703..517d0d5f 100644 --- a/scripts/assemble-changes.mjs +++ b/scripts/assemble-changes.mjs @@ -66,16 +66,18 @@ export function ciFiles() { } /** - * One key over every CI definition path, matched against the per-file keys to catch a file that - * `ciFiles()` cannot see — see `hasUnattributedCiFile`. + * One key over every CI definition path, whose matched **paths** — not just the key — come back + * to `assemble` via `list-files: json`, to catch a file that `ciFiles()` cannot see — see + * `unclaimedCiPaths`. */ const CI_ANY = 'ci__any' /** * Matches only when a CI definition file was **deleted** — a paths-filter status predicate over - * the same two globs as `CI_ANY`, so it fires on the deletion itself, no matter what else changed - * alongside it. This is the signal no key heuristic can reconstruct: matched keys cannot tell a - * deletion beside an edit from the edit alone. + * the same two globs as `CI_ANY`, firing on the deletion itself no matter what else changed + * alongside it. A deleted path also surfaces through `unclaimedCiPaths`, but that rests on the + * file list arriving intact; this fires from the filter match alone, so the deletion case never + * hangs on one mechanism. */ const CI_DELETED = 'ci__deleted' @@ -97,20 +99,24 @@ export function buildFilters() { } /** - * True when a CI definition file changed that no `wf__` key claimed. The per-file keys only cover - * what `ciFiles()` names, so a CI file outside that set matches the catch-all and nothing else: - * without this, a pull request changing only such files runs no job at all and reports green. It - * validates everything — the same answer an unreadable marker gets, for the same reason: this - * must over-run, never quietly under-run. + * The changed CI paths that no per-file key claims: `ci__any`'s matches, minus `ciFiles()` and + * minus the `.md` companions, which are documentation no job reads. Anything left is a CI file + * the attribution cannot see — a deleted workflow (the keys are built from the working tree), an + * aux file inside an action — and the caller validates everything for it: the same answer an + * unreadable marker gets, for the same reason: this must over-run, never quietly under-run. * - * Blind past the first claim, though: from matched keys alone, a deletion beside an edit looks - * exactly like the edit by itself, and the edit's claim can be far narrower than what the deleted - * file validated. Deletions are therefore caught by `CI_DELETED` — a status predicate, not a key - * heuristic — and this covers only what that cannot: a changed file that still exists but carries - * no key. + * Paths rather than keys, because keys cannot say this. A deletion or an unkeyed file beside an + * ordinary edit produces the same matched keys as the edit alone, so any key heuristic goes blind + * past the first claim — while the edit's claim can be far narrower than what the masked file + * validated. `null` means `ci__any` matched but the file list itself is missing: nothing can be + * ruled out, and the caller treats it as "everything" too. */ -export const hasUnattributedCiFile = (changed) => - changed.includes(CI_ANY) && !changed.some((key) => key.startsWith('wf__')) +export function unclaimedCiPaths(changed, files) { + if (!changed.includes(CI_ANY)) return [] + if (files === null) return null + const claimed = new Set(Object.values(ciFiles())) + return files.filter((path) => !claimed.has(path) && !path.endsWith('.md')) +} /** * The importer blocks of a pnpm lockfile, keyed by workspace path. Sectioned by indentation @@ -466,10 +472,24 @@ function main() { const workflows = attributeWorkflows( changed.filter((key) => key.startsWith('wf__')).map((key) => key.slice('wf__'.length)) ) - const removedCiFile = changed.includes(CI_DELETED) || hasUnattributedCiFile(changed) + // `ci__any`'s matched paths, laid alongside CHANGES by the same paths-filter step. Missing or + // unreadable stays null: `unclaimedCiPaths` then rules nothing out. + let ciAnyFiles = null + try { + const parsed = JSON.parse(process.env.CI_ANY_FILES) + if (Array.isArray(parsed)) ciAnyFiles = parsed + } catch { + /* null already says it */ + } + const unclaimed = unclaimedCiPaths(changed, ciAnyFiles) + const unattributedCi = changed.includes(CI_DELETED) || unclaimed === null || unclaimed.length > 0 const reasons = [...root.reasons, ...workflows.reasons] if (changed.includes(CI_DELETED)) reasons.push('a CI file was deleted — nothing left to claim it') - else if (removedCiFile) reasons.push('a CI file changed that no per-file key claimed') + if (unclaimed === null) { + reasons.push('a CI file changed and the matched paths could not be read') + } else if (unclaimed.length > 0) { + reasons.push(`CI files changed that no per-file key claims — ${unclaimed.join(', ')}`) + } for (const reason of reasons) console.error(`changes: ${reason}`) const { changes, outputs } = assembleChanges({ @@ -477,7 +497,7 @@ function main() { members, attributed: root.attributed, revalidate: workflows.keys, - revalidateAll: workflows.wholeWorkspace || removedCiFile, + revalidateAll: workflows.wholeWorkspace || unattributedCi, wholeWorkspace: root.wholeWorkspace, })