From ebd11762ad575ab4181460c3ba85685816a23577 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Sun, 24 May 2026 16:07:41 +0100 Subject: [PATCH] fix(webapp): retain sessions-replication singleton import via globalThis assignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `apps/webapp/package.json` declares `"sideEffects": false`. PR #3333 (commit 71d98b4e) replaced the previous real method-call retention idiom at the two singleton import sites with: void sessionsReplicationInstance; esbuild treats `void ;` as a pure expression statement under `sideEffects: false` and tree-shakes the entire import — including the `singleton(...)` call inside `sessionsReplicationInstance.server.ts` which is the only thing that fires `initializeSessionsReplicationInstance()`. The sessions→ClickHouse logical replication worker never starts, the slot is unconsumed, lag grows. Verified empirically: grep of the built bundle `apps/webapp/build/index.js` (from c0365d36) returns 0 occurrences of `Sessions replication` / `sessionsReplicationInstance` / `SessionsReplicationService`, vs 3 occurrences of the runs-replication strings. The runs path survives because `adminWorker.server.ts` and `admin.api.v1.runs-replication.*` routes have real method calls (.start(), .teardown(), .backfill()) that the tree-shaker must preserve. Fix: replace the `void` with an assignment to `globalThis`, which is an unambiguous observable side effect the bundler cannot eliminate. Update the surrounding comments at both sites to document the bundler interaction so the next maintainer doesn't reintroduce `void`. Incident: TRI-9864. Mitigated in production by reverting the image (cloud#910 / TRI-9863); this PR is the forward fix that lets the original image bump land safely. Follow-ups worth filing separately: - Change apps/webapp/package.json's "sideEffects": false to an allowlist that includes *Instance.server.ts files, so the same shape of regression in any future *Instance singleton is structurally prevented. - Add a build-time grep check (e.g. require 'Sessions replication' to appear in apps/webapp/build/index.js) to publish.yml so this exact regression fails CI. Refs TRI-9864, TRI-9863. --- apps/webapp/app/entry.server.tsx | 11 ++++++++++- apps/webapp/app/v3/services/adminWorker.server.ts | 12 +++++++++--- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index db72b0364c2..60c234402d5 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -30,8 +30,17 @@ import { // on webapp startup. The singleton's initializer wires start (gated on // `clickhouseFactory.isReady()`) and SIGTERM/SIGINT shutdown — mirrors // runsReplicationInstance. +// +// IMPORTANT: do NOT replace this with `void sessionsReplicationInstance;`. +// `apps/webapp/package.json` declares `"sideEffects": false`, so esbuild +// treats `void ;` as a pure expression statement and tree-shakes +// the entire import — the singleton's initializer never fires and the +// sessions→ClickHouse logical replication slot stops being consumed. Assigning +// to globalThis is an unambiguous side effect the bundler must preserve. See +// TRI-9864 for the incident write-up. import { sessionsReplicationInstance } from "./services/sessionsReplicationInstance.server"; -void sessionsReplicationInstance; +(globalThis as Record).__sessionsReplicationInstance = + sessionsReplicationInstance; const ABORT_DELAY = 30000; diff --git a/apps/webapp/app/v3/services/adminWorker.server.ts b/apps/webapp/app/v3/services/adminWorker.server.ts index 2e4d1b066cb..cf3dbd57c6c 100644 --- a/apps/webapp/app/v3/services/adminWorker.server.ts +++ b/apps/webapp/app/v3/services/adminWorker.server.ts @@ -6,10 +6,16 @@ import { logger } from "~/services/logger.server"; import { runsReplicationInstance } from "~/services/runsReplicationInstance.server"; // Reference-hold the sessions-replication singleton so module evaluation runs // its initializer (creates the ClickHouse client, subscribes to the logical -// replication slot, wires signal handlers) when the webapp boots. A bare -// side-effect import gets tree-shaken by the bundler. +// replication slot, wires signal handlers) when the webapp boots. +// +// IMPORTANT: do NOT replace with `void sessionsReplicationInstance;`. With +// `"sideEffects": false` in apps/webapp/package.json, esbuild treats `void X;` +// as a pure expression statement and eliminates the import — the singleton +// initializer never fires. Assignment to globalThis is an observable side +// effect the bundler must preserve. See TRI-9864. import { sessionsReplicationInstance } from "~/services/sessionsReplicationInstance.server"; -void sessionsReplicationInstance; +(globalThis as Record).__sessionsReplicationInstance = + sessionsReplicationInstance; import { singleton } from "~/utils/singleton"; import { tracer } from "../tracer.server"; import { $replica } from "~/db.server";