From e3268735f296c5d31db8ddf5feedcbf5d8da7f25 Mon Sep 17 00:00:00 2001 From: Maurice Date: Tue, 20 Oct 2026 08:35:41 +0200 Subject: [PATCH] ci: fold the gate jobs into two per package, so the long jobs fit the 20 runners With three gate jobs per package a code change started ~29 jobs, 22 of them running for minutes, against the organisation's 20 concurrent jobs. Two test shards waited three minutes for a slot and the run took 9.9 minutes instead of 8.5. Server lint and checks, and client types and checks, now share a job each (about 4-5 minutes, still shorter than a shard), which leaves 20 long jobs. The note at the top of test.yml says to count them before adding one. --- .github/workflows/test.yml | 190 ++++++++++++++--------------------- CLAUDE.md | 4 +- scripts/ci/changed-areas.mjs | 2 +- 3 files changed, 76 insertions(+), 120 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index ee8f2a547..d4219ea66 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -19,8 +19,8 @@ permissions: # the server's src, tests and scripts, smokes the # production require chain, the strict ratchet, # the generated entities and Kysely types -# Server: Lint ESLint (typed), errors and warning counts -# Server: Checks Prettier, the plugin facts, the DB call graph, +# Server: Lint & Checks ESLint (typed), errors and warning counts; then +# Prettier, the plugin facts, the DB call graph, # the shared ratchet code, the source ratchets # (size, boundaries, dialect, tx, test SQL, test # mocks, hand-built services, query API, MCP zod, @@ -32,12 +32,12 @@ permissions: # Server: S3 Contract (MinIO) # the storage driver against MinIO # Server: Postgres Probe the repository SQL against Postgres -# Client: Types typecheck and the strict ratchet +# Client: Types & Checks typecheck and the strict ratchet; then the page +# pattern and the source ratchets (RTL, size, +# Prettier, layers, offline path, duplication, +# desktop/phone pairs, skips, i18n keys, theme +# tokens) # Client: Lint ESLint (typed), errors and warning counts -# Client: Checks the page pattern and the source ratchets (RTL, -# size, Prettier, layers, offline path, -# duplication, desktop/phone pairs, skips, i18n -# keys, theme tokens) # Client: Tests n/7 one slice of the client suite, with coverage # Client: Coverage merges the slices, checks the thresholds, # uploads lcov @@ -50,25 +50,28 @@ permissions: # ci-ok the one required check (below) # # Job layout: -# - Everything starts right after changes: the gates (types, lint, checks) -# and the tests run side by side, so a run takes as long as its slowest -# job and not the gates plus the tests. The tests used to wait for the -# gates, which put ~9 minutes in front of every test result, and the -# gates were one job per package (~8-9 minutes each) before they were -# split into types, lint and checks. +# - Everything starts right after changes: the gates and the tests run side +# by side, so a run takes as long as its slowest job and not the gates +# plus the tests. The tests used to wait for the gates, which put ~9 +# minutes in front of every test result. The gates were one job per +# package (~8-9 minutes each); each is now two jobs of ~4-5 minutes, +# split where the time is (the server's typechecks, the client's lint), +# so neither outlasts a test shard. # - The client and server suites run in shards (client-tests, # server-tests), each running a slice with coverage and uploading a blob # report; client-coverage and server-coverage merge the blobs, check the # thresholds against the whole and upload lcov. A red shard is usually a # failing test, a red merge job usually a threshold. None of the other # test jobs feeds the scan. -# - Shard counts: GitHub runs at most 20 jobs of the organisation at once. -# A code change starts about fifteen jobs besides the shards; about half -# of them (parity, the SDK, the bundle, S3, Postgres, shared coverage, -# the chart) are done within a minute or two, so the shards that queue at -# the start wait about that long. The counts are sized so the coverage -# merges finish about when the slowest gate does; more shards would wait -# in the queue rather than finish sooner. +# - Job count: GitHub runs at most 20 jobs of the organisation at once and +# does not say in which order it starts queued ones. A code change starts +# the 14 shards and about a dozen other jobs; seven of those (parity, the +# SDK, the bundle, S3, Postgres, shared coverage, the chart) are done in +# a minute or two, so what queues at the start waits about that long. The +# jobs that run for minutes (shards, gates, e2e, the image) must not +# outnumber the 20 slots, or one of them waits for another to finish and +# the run grows by a whole job: a layout with six gate jobs did exactly +# that. Check the count before adding a long job or a shard. # - Scan `needs:` the gates and the four coverage jobs. A coverage job # failing must stop the scan — a missing lcov file would make every line # of that package report as uncovered, and the dashboard would show a @@ -276,8 +279,8 @@ jobs: # migration that changes a column regenerates it in the same change. run: cd server && npm run check:db-types - server-lint: - name: 'Server: Lint' + server-lint-checks: + name: 'Server: Lint & Checks' needs: changes if: needs.changes.outputs.code == 'true' runs-on: ubuntu-24.04 @@ -316,27 +319,6 @@ jobs: TREK_LINT_BASE: refs/remotes/lint-base run: cd server && npm run lint:warnings - server-checks: - name: 'Server: Checks' - needs: changes - if: needs.changes.outputs.code == 'true' - runs-on: ubuntu-24.04 - timeout-minutes: 45 - steps: - - uses: actions/checkout@v6 - - - uses: actions/setup-node@v6 - with: - node-version-file: .nvmrc - cache: npm - cache-dependency-path: package-lock.json - - - name: Install dependencies - run: npm ci - - - name: Build shared - run: npm run build --workspace=shared - # The plugin permission facts live once, in # server/src/nest/plugins/protocol/envelope.ts. Every other copy — the # SDK's generated/host-facts.ts and shared's plugin-permissions.ts — is @@ -440,12 +422,12 @@ jobs: # knip (knip.jsonc) over client, server and shared: unused files, # exports, types and dependencies per file may not rise above # scripts/ci/knip-baseline.json (scripts/ci/knip-ratchet.mjs). Here - # rather than in client-checks because it needs every workspace's + # rather than in client-types-checks because it needs every workspace's # dependencies, which the client jobs do not install. run: npm run lint:knip - client-types: - name: 'Client: Types' + client-types-checks: + name: 'Client: Types & Checks' needs: changes if: needs.changes.outputs.code == 'true' runs-on: ubuntu-24.04 @@ -476,69 +458,6 @@ jobs: # one included, has to be clean (scripts/strict-lint.mjs). run: cd client && npm run lint:strict - client-lint: - name: 'Client: Lint' - needs: changes - if: needs.changes.outputs.code == 'true' - runs-on: ubuntu-24.04 - timeout-minutes: 45 - - steps: - - uses: actions/checkout@v6 - - - uses: actions/setup-node@v6 - with: - node-version-file: .nvmrc - cache: npm - cache-dependency-path: package-lock.json - - - name: Install dependencies - run: npm ci --workspace shared --workspace client - - - name: Build shared - run: npm run build --workspace=shared - - - name: Fetch the base the warnings are compared with - # lint:warnings names the file behind a grown count by linting it as - # it is on the base: the pushed-over commit on a push, the target - # branch on a pull request. A miss only drops that hint. - run: >- - git fetch --no-tags --depth=1 origin - "+${{ github.event_name == 'push' && github.event.before || format('refs/heads/{0}', github.base_ref || 'dev') }}:refs/remotes/lint-base" - || true - - - name: Lint - # Fails on any ESLint error, like lint:check did, and on any rule - # whose warning count rises above scripts/eslint-baseline.json. - # Typed linting (no-floating-promises, no-misused-promises) loads the - # whole TypeScript program, so it gets the heap the server lint gets. - env: - NODE_OPTIONS: --max-old-space-size=8192 - TREK_LINT_BASE: refs/remotes/lint-base - run: cd client && npm run lint:warnings - - client-checks: - name: 'Client: Checks' - needs: changes - if: needs.changes.outputs.code == 'true' - runs-on: ubuntu-24.04 - timeout-minutes: 45 - - steps: - - uses: actions/checkout@v6 - - - uses: actions/setup-node@v6 - with: - node-version-file: .nvmrc - cache: npm - cache-dependency-path: package-lock.json - - - name: Install dependencies - run: npm ci --workspace shared --workspace client - - - name: Build shared - run: npm run build --workspace=shared - - name: Page pattern check run: cd client && npm run lint:pages @@ -602,6 +521,47 @@ jobs: # follow the user's scheme, accent and text size (scripts/theme-lint.mjs). run: cd client && npm run theme:lint + client-lint: + name: 'Client: Lint' + needs: changes + if: needs.changes.outputs.code == 'true' + runs-on: ubuntu-24.04 + timeout-minutes: 45 + + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-node@v6 + with: + node-version-file: .nvmrc + cache: npm + cache-dependency-path: package-lock.json + + - name: Install dependencies + run: npm ci --workspace shared --workspace client + + - name: Build shared + run: npm run build --workspace=shared + + - name: Fetch the base the warnings are compared with + # lint:warnings names the file behind a grown count by linting it as + # it is on the base: the pushed-over commit on a push, the target + # branch on a pull request. A miss only drops that hint. + run: >- + git fetch --no-tags --depth=1 origin + "+${{ github.event_name == 'push' && github.event.before || format('refs/heads/{0}', github.base_ref || 'dev') }}:refs/remotes/lint-base" + || true + + - name: Lint + # Fails on any ESLint error, like lint:check did, and on any rule + # whose warning count rises above scripts/eslint-baseline.json. + # Typed linting (no-floating-promises, no-misused-promises) loads the + # whole TypeScript program, so it gets the heap the server lint gets. + env: + NODE_OPTIONS: --max-old-space-size=8192 + TREK_LINT_BASE: refs/remotes/lint-base + run: cd client && npm run lint:warnings + client-bundle: name: 'Client: Bundle' needs: changes @@ -1097,11 +1057,9 @@ jobs: needs: - i18n-parity - server-types - - server-lint - - server-checks - - client-types + - server-lint-checks + - client-types-checks - client-lint - - client-checks - server-coverage - client-coverage - shared-coverage @@ -1316,11 +1274,9 @@ jobs: - changes - i18n-parity - server-types - - server-lint - - server-checks - - client-types + - server-lint-checks + - client-types-checks - client-lint - - client-checks - client-bundle - client-e2e - s3-contract diff --git a/CLAUDE.md b/CLAUDE.md index d0bf611a8..8b6cf1a0a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -134,9 +134,9 @@ These principles come out of a full-repo audit and shape all new code: ## Quality gates in CI -`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`, client also the strict-mode ratchet `lint:strict`), lint (client and server through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:strict`, `lint:boundaries`, `lint:tx`, `lint:dialect`, `lint:test-sql`, `lint:test-mocks`, `lint:test-new-service`, `lint:query-api`, `lint:mcp-zod`, `lint:service-http` and `lint:response-contracts`, the client's and server's `lint:format`, `lint:layers`, `lint:offline`, `lint:dup`, `lint:pairs`, `lint:skips`, `lint:i18n-keys` and `theme:lint`, shared's `contracts:open`, the repo-wide `lint:knip` (in server-checks), `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and the statements every repository method reaches with sample arguments against a Postgres service; the SQL errors per method and the list of methods it cannot measure are held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `lint:format` on `shared/`, both check-only. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags. +`.github/workflows/test.yml` runs, per package: typecheck (server also `typecheck:tests`, client also the strict-mode ratchet `lint:strict`), lint (client and server through `lint:warnings`, which also holds the warning count per rule), `lint:pages`, `lint:rtl`, `lint:size` (client and server), the server's `lint:strict`, `lint:boundaries`, `lint:tx`, `lint:dialect`, `lint:test-sql`, `lint:test-mocks`, `lint:test-new-service`, `lint:query-api`, `lint:mcp-zod`, `lint:service-http` and `lint:response-contracts`, the client's and server's `lint:format`, `lint:layers`, `lint:offline`, `lint:dup`, `lint:pairs`, `lint:skips`, `lint:i18n-keys` and `theme:lint`, shared's `contracts:open`, the repo-wide `lint:knip` (in server-lint-checks), `check:plugin-facts`, `i18n:parity:strict` (with the untranslated-strings ratchet), the S3 contract suite, the Postgres probe (`postgres-probe`: every dialect helper on SQLite and Postgres, and the statements every repository method reaches with sample arguments against a Postgres service; the SQL errors per method and the list of methods it cannot measure are held to a baseline that only shrinks and fails while it is still unmeasured; see `server/CLAUDE.md`), the browser end-to-end flows (Chromium and WebKit), a client build with `check:gl-split`, coverage for all four packages, then a SonarCloud scan. It also builds the Docker image and boots it until `/api/health/ready` answers (`docker-image`), and lints and renders the Helm chart against the Kubernetes schemas and runs `docker compose config` on every compose file (`deploy-manifests`). `lint-prettier.yml` additionally runs `lint` + `lint:format` on `shared/`, both check-only. `plugin-sdk` has no lint script and is outside the eslint gates. **Not in CI**: the screenshot and help-media Playwright projects; run them locally. There are no git hooks; nothing runs before a commit except you. Other workflows build Docker images, scan them, and publish `trek-plugin-sdk` on `plugin-sdk-v*` tags. -- **Job names read "package: what"** (`Server: Lint`, `Client: Tests 3/7`), and the list at the top of `test.yml` says what each job holds. A new job follows the scheme and joins that list. +- **Job names read "package: what"** (`Server: Types`, `Client: Tests 3/7`), and the list at the top of `test.yml` says what each job holds. A new job follows the scheme and joins that list, and the job-count note there says when a long job needs a slot freed first. - **Tests run beside the gates, in shards.** The client and server suites run in slices on parallel runners (`client-tests`, `server-tests`, through each package's `vitest.shard.config.ts`), and `client-coverage`/`server-coverage` merge their blob reports and check the thresholds against the whole. A red slice is usually a failing test, a red merge job usually a threshold. - **`ci-ok` is the required check.** The branch ruleset requires `ci-ok` (plus `lint` and `check-target` from their own workflows), and `ci-ok` needs every job above except the scan. It fails if any of them failed or was cancelled and passes if they passed or were skipped. A new gate job counts only once it is listed in `ci-ok`'s `needs:`. - **Guides name paths that exist**: the `changes` job runs `scripts/ci/doc-paths.mjs`, which resolves every backtick path in the CLAUDE.md files, `README.md`, `server/src/nest/README.md` and the wiki against the tracked files; the missing ones are listed in `scripts/ci/doc-paths-baseline.json`, which only shrinks (`--update` after a fix). diff --git a/scripts/ci/changed-areas.mjs b/scripts/ci/changed-areas.mjs index e6192e124..ab3b5befd 100644 --- a/scripts/ci/changed-areas.mjs +++ b/scripts/ci/changed-areas.mjs @@ -49,7 +49,7 @@ export const AREAS = { 'package-lock.json', '.nvmrc', 'sonar-project.properties', - // lint:knip in server-checks reads it. + // lint:knip in server-lint-checks reads it. 'knip.jsonc', 'Dockerfile', ...READ_BY_TESTS,