fix(webapp): retain sessions-replication singleton import via globalThis assignment#3738
Conversation
…his assignment `apps/webapp/package.json` declares `"sideEffects": false`. PR #3333 (commit 71d98b4) replaced the previous real method-call retention idiom at the two singleton import sites with: void sessionsReplicationInstance; esbuild treats `void <identifier>;` 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 c0365d3) 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.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
🧰 Additional context used📓 Path-based instructions (8)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.ts📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Files:
apps/webapp/**/*.server.ts📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
**/*.{js,jsx,ts,tsx,json,md,yml,yaml}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/webapp/**/*.{tsx,jsx}📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Files:
🧠 Learnings (11)📚 Learning: 2026-03-10T17:56:20.938ZApplied to files:
📚 Learning: 2026-03-22T13:26:12.060ZApplied to files:
📚 Learning: 2026-03-22T19:24:14.403ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-05-18T08:21:27.694ZApplied to files:
📚 Learning: 2026-03-29T19:16:28.864ZApplied to files:
📚 Learning: 2026-05-05T09:38:02.512ZApplied to files:
📚 Learning: 2026-05-12T21:04:05.815ZApplied to files:
📚 Learning: 2026-05-14T08:21:07.614ZApplied to files:
📚 Learning: 2026-02-11T16:37:32.429ZApplied to files:
📚 Learning: 2026-05-08T21:00:20.973ZApplied to files:
🔇 Additional comments (2)
WalkthroughThis pull request fixes bundler tree-shaking of the sessions replication singleton initializer across two server-side module entry points. The prior pattern— Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Linear: TRI-9864 (Urgent)
Production incident: TRI-9863 (mitigated by image revert in cloud#910)
Bug
apps/webapp/package.jsondeclares"sideEffects": false. PR #3333 (71d98b4e) replaced the previous real method-call retention idiom at the twosessionsReplicationInstanceimport sites with:esbuild treats
void <identifier>;as a pure expression statement undersideEffects: falseand tree-shakes the entire import — including thesingleton(...)call insidesessionsReplicationInstance.server.tswhich is the only thing that firesinitializeSessionsReplicationInstance(). The sessions→ClickHouse logical replication worker never starts, the slot is unconsumed, lag grows.How it manifested in production
cloud#907's image bump rolled the
SessionReplicationServiceECS task on prod at 14:32 UTC. The new container's startup log emitted🗃️ Runs replication service enabledbut not🗃️ Sessions replication service enabledor🗃️ Sessions replication service started. CloudWatchOldestReplicationSlotLaggrew at ~220 MB/min and theHigh replication lagalarm fired at 14:37 UTC. Prod was reverted to the previous image (cloud#910) to stop the bleed.Verification
grepof the built bundleapps/webapp/build/index.js(built fromc0365d36):Runs replication/runsReplicationInstancestrings ✅Sessions replication/sessionsReplicationInstance/SessionsReplicationService❌The runs path survives tree-shaking because
adminWorker.server.tsandadmin.api.v1.runs-replication.*routes have real method calls (.start(),.teardown(),.backfill()) — observable uses the tree-shaker must preserve. The sessions singleton has no real callers, only thevoidno-ops, hence its complete elimination from the bundle.Fix
Replace
void sessionsReplicationInstance;with an assignment toglobalThis, an unambiguous observable side effect the bundler cannot eliminate:Applied at both call sites:
apps/webapp/app/entry.server.tsxandapps/webapp/app/v3/services/adminWorker.server.ts.Surrounding comments updated to document the bundler interaction so the next maintainer doesn't reintroduce
void.Out of scope (follow-ups)
apps/webapp/package.jsonfrom"sideEffects": falseto an allowlist that includes*Instance.server.tsfiles. Prevents the same regression shape via any future*Instancesingleton.greppost-build step inpublish.ymlrequiring"Sessions replication"to appear inapps/webapp/build/index.js. Catches this exact regression at CI time.Test plan
pnpm run typecheck --filter webappcleanSessionReplicationServicecontainer logs🗃️ Sessions replication service enabledand🗃️ Sessions replication service startedat startupOldestReplicationSlotLagstops growing and drains