From e389567a3c4909f6b058816a8ae96099d701d21b Mon Sep 17 00:00:00 2001 From: Julien Gautier Date: Wed, 20 May 2026 15:06:25 +0200 Subject: [PATCH] fix(ci): move grpc-tools to optionalDependencies (revert NODE_OPTIONS attempt) (#200) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary The `NODE_OPTIONS=--use-system-ca` workaround merged in #199 did not clear the CI install failure — the runner still rejects the TLS chain to `node-precompiled-binaries.grpc.io`. Either the runner's OS CA bundle is missing the same intermediate Node's bundled set is missing, or `--use-system-ca` does not propagate down to the `node-fetch@2` that `node-pre-gyp` uses. Without runner shell access the exact reason is not worth chasing. Switch angle: **the protoc binary `grpc-tools` downloads is never used in CI**. The generated TypeScript stubs in `apps/portal-bff/src/grpc/gen/` are committed per [ADR-0024](docs/decisions/0024-ai-service-relay-grpc-sse-bridge.md) §"Sub-decision 3 — vendored protos", so CI only needs to type-check them; protoc only runs when a developer regenerates from `apps/portal-bff/src/grpc/proto/apf-ai/*.proto`. This PR moves `grpc-tools` from `devDependencies` to `optionalDependencies`. Per pnpm semantics, when an optional dep's install (including postinstall) fails, pnpm logs a warning and the overall install completes. CI's `pnpm install --frozen-lockfile` therefore succeeds even when the binary cannot be fetched; developer machines (where TLS validates) install grpc-tools normally and `pnpm grpc:codegen` works unchanged. ## What lands - `package.json` — `grpc-tools: "^1.13.0"` moves out of `devDependencies` and into a new `optionalDependencies` block. The entry in `pnpm.onlyBuiltDependencies` stays — it controls whether pnpm *attempts* the postinstall, not whether failure is fatal. Local installs (where TLS works) still run the postinstall and download the binary. - `pnpm-lock.yaml` — refreshed; the lockfile reflects the new optional-dep classification. No version changes to anything else. - `.gitea/workflows/ci.yml` — revert the workflow-level `env: NODE_OPTIONS: --use-system-ca` block added in #199. Did not help; left in place it would be a misleading "this is supposed to fix CI TLS" signpost. - `.gitea/workflows/docs-site.yml` — same revert. ## Notes for the reviewer - **Why `optionalDependencies` is the right primitive.** pnpm 10 documents the contract: a package listed in `optionalDependencies` is *attempted*; if the install fails for any reason (platform mismatch, postinstall script error, network failure), the failure is logged and the overall command continues. That maps exactly to what this PR needs: try in CI, fail gracefully, succeed locally. - **Why not remove `grpc-tools` from `pnpm.onlyBuiltDependencies` instead.** Removing it from the allowlist tells pnpm not to even *try* the postinstall, which would also fix CI. But it would also break the developer workflow — locally, the protoc binary would never download, and `pnpm grpc:codegen` would fail. The dev would have to manually trigger the build (`pnpm approve-builds` then `pnpm rebuild grpc-tools`), or worse, devs would commit accidental `package.json` mutations from `approve-builds`. The `optionalDependencies` route keeps the local DX identical. - **Why revert the workflow `env:` blocks.** They do not help here, and leaving them in place implies that `--use-system-ca` is the canonical fix for this class of failure — which it is *not*, at least not for this runner / this CDN. If a future native dep hits a similar wall and `--use-system-ca` *does* fix it for that case, the env var lands in a focused PR with a real validation. Carrying it now as a "maybe useful later" workaround is noise. - **Codegen workflow on developer machines.** Unchanged. `pnpm install` runs the postinstall (TLS validates locally), the protoc binary lands in `node_modules/`, `pnpm grpc:codegen` regenerates stubs. The only visible difference is a one-line `WARN GET_RESOLVED_FROM_REGISTRY ...` if a contributor ever encounters the same TLS failure locally (e.g., on a corporate-proxied machine) — pnpm will mark grpc-tools as failed-optional and the rest of the install proceeds; the dev can then debug their own network without blocking the whole repo. - **Idempotence with CI's `--frozen-lockfile`.** The lockfile change is small (re-classifies grpc-tools from `dev` to `optional`) and lands in this PR. After merge, CI runs against the new lockfile; no further coordination needed. ## Test plan - [x] `pnpm install` locally — clean, grpc-tools postinstall runs successfully (TLS validates), protoc binary present. - [x] `pnpm grpc:codegen` — regenerates the TypeScript stubs identically (no diff in `apps/portal-bff/src/grpc/gen/`). - [x] `pnpm nx run-many -t lint test build -p portal-shell,portal-admin,portal-bff,shared-ui,shared-charts` — all five projects green. - [ ] **CI green on this PR's first run.** The validation that matters: `pnpm install --frozen-lockfile` completes despite grpc-tools' postinstall failure; `ci:check` runs to completion. - [ ] Spot-check of post-merge CI runs over the next few days to confirm the warning is recurring (expected) and the install never fails (the contract). ## What's next - If the CI behaviour is what this PR predicts (warning instead of failure), no further action required. - If `optionalDependencies` does not work as documented (highly unlikely, but possible if the pnpm version has a regression), the fallback is to drop `grpc-tools` from `pnpm.onlyBuiltDependencies` and add a small dev-onboarding note. One-line change away. - Long-term cleanup: if a future PR migrates the codegen step to a Docker-based tool (`buf`, system protoc) the optional-dep entry can be removed entirely. Out of scope here. --------- Co-authored-by: Julien Gautier Reviewed-on: https://git.unespace.com/julien/apf_portal/pulls/200 --- .gitea/workflows/ci.yml | 12 ------------ .gitea/workflows/docs-site.yml | 7 ------- package.json | 4 +++- pnpm-lock.yaml | 13 +++++++++---- 4 files changed, 12 insertions(+), 24 deletions(-) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index 1bd0753..7123a19 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -11,18 +11,6 @@ on: push: branches: [main] -# Node 24's bundled CA set does not include every intermediate the -# self-hosted runner's network path serves (the precompiled-binary -# CDN behind grpc-tools is the first surface where this surfaced). -# `--use-system-ca` tells Node to consult the OS CA store in -# addition to its bundled set — supported since Node 22 and the -# documented Node-team fix for exactly this class of TLS-chain -# rejection. The runner image (`catthehacker/ubuntu:act-22.04` per -# ADR-0015) carries the standard Ubuntu `ca-certificates` bundle, -# which validates the impacted chains. -env: - NODE_OPTIONS: --use-system-ca - jobs: check: runs-on: [self-hosted, on-prem] diff --git a/.gitea/workflows/docs-site.yml b/.gitea/workflows/docs-site.yml index 113bbce..1d271e1 100644 --- a/.gitea/workflows/docs-site.yml +++ b/.gitea/workflows/docs-site.yml @@ -28,13 +28,6 @@ on: - 'pnpm-lock.yaml' - '.gitea/workflows/docs-site.yml' -# See `.gitea/workflows/ci.yml` for the rationale — Node 24's -# bundled CA set rejects some chains the runner's OS trust store -# validates, and `--use-system-ca` (Node ≥ 22) tells Node to -# consult the OS CA bundle alongside its own. -env: - NODE_OPTIONS: --use-system-ca - jobs: build: runs-on: [self-hosted, on-prem] diff --git a/package.json b/package.json index 22c0363..50f2282 100644 --- a/package.json +++ b/package.json @@ -111,7 +111,6 @@ "eslint": "^9.8.0", "eslint-config-prettier": "^10.0.0", "eslint-plugin-playwright": "^1.6.2", - "grpc-tools": "^1.13.0", "husky": "^9.1.7", "jest": "^30.0.2", "jest-environment-node": "^30.0.2", @@ -196,5 +195,8 @@ "pino-http": "^11.0.0", "reflect-metadata": "^0.2.0", "rxjs": "~7.8.0" + }, + "optionalDependencies": { + "grpc-tools": "^1.13.0" } } diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 15828a4..63839d3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -356,9 +356,6 @@ importers: eslint-plugin-playwright: specifier: ^1.6.2 version: 1.8.3(eslint@9.39.4(jiti@2.7.0)) - grpc-tools: - specifier: ^1.13.0 - version: 1.13.1(encoding@0.1.13) husky: specifier: ^9.1.7 version: 9.1.7 @@ -434,6 +431,10 @@ importers: webpack-cli: specifier: ^5.1.4 version: 5.1.4(webpack@5.106.2) + optionalDependencies: + grpc-tools: + specifier: ^1.13.0 + version: 1.13.1(encoding@0.1.13) libs/shared/tokens: dependencies: @@ -13769,6 +13770,7 @@ snapshots: transitivePeerDependencies: - encoding - supports-color + optional: true '@mermaid-js/mermaid-mindmap@9.3.0': dependencies: @@ -17052,7 +17054,8 @@ snapshots: dependencies: argparse: 2.0.1 - abbrev@3.0.1: {} + abbrev@3.0.1: + optional: true abbrev@4.0.0: {} @@ -19242,6 +19245,7 @@ snapshots: transitivePeerDependencies: - encoding - supports-color + optional: true hachure-fill@0.5.2: {} @@ -20863,6 +20867,7 @@ snapshots: nopt@8.1.0: dependencies: abbrev: 3.0.1 + optional: true nopt@9.0.0: dependencies: