Uh oh!
There was an error while loading. Please reload this page.
fix(ai-event-client): defer devtools runtime-id generation to first use - #683
Conversation
The runtime id was seeded at module scope via a top-level IIFE calling crypto.randomUUID() / Math.random(). Because @tanstack/ai's chat() always pulls in the devtools middleware, this random-value generation ran at module-evaluation time. Edge runtimes such as Cloudflare Workers forbid generating random values in global scope, so importing chat() crashed the Worker with "Disallowed operation called within global scope". Generate the runtime id lazily on first use (inside getAIDevtoolsRuntimeId() / createAIDevtoolsEventEnvelope()) and memoize it, so evaluating the module performs no random-value generation. The cross-bundle global and the generated values are unchanged. Fixes#667. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR defers devtools runtime-id generation from module-load time to first function call by replacing eager, top-level initialization with a memoized ChangesLazy runtime-id initialization
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
🚀 Changeset Version Preview1 package(s) bumped directly, 18 bumped as dependents. 🟩 Patch bumps
|
View your CI Pipeline Execution ↗ for commit 144a376
☁️ Nx Cloud last updated this comment at |
@tanstack/ai@tanstack/ai-anthropic@tanstack/ai-client@tanstack/ai-code-mode@tanstack/ai-code-mode-skills@tanstack/ai-devtools-core@tanstack/ai-elevenlabs@tanstack/ai-event-client@tanstack/ai-fal@tanstack/ai-gemini@tanstack/ai-grok@tanstack/ai-groq@tanstack/ai-isolate-cloudflare@tanstack/ai-isolate-node@tanstack/ai-isolate-quickjs@tanstack/ai-ollama@tanstack/ai-openai@tanstack/ai-openrouter@tanstack/ai-preact@tanstack/ai-react@tanstack/ai-react-ui@tanstack/ai-solid@tanstack/ai-solid-ui@tanstack/ai-svelte@tanstack/ai-utils@tanstack/ai-vue@tanstack/ai-vue-ui@tanstack/openai-base@tanstack/preact-ai-devtools@tanstack/react-ai-devtools@tanstack/solid-ai-devtoolscommit: |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/ai-event-client/tests/runtime-id.test.ts (1)
87-112: 💤 Low valueConsider improving crypto restoration logic.
The current cleanup restores
cryptoonly whencryptoDescriptoris truthy. Ifcryptodidn't exist originally onglobalThis(unlikely in modern environments, but possible),cryptoDescriptorwould beundefined, and the finally block would leavecryptoset toundefinedinstead of deleting the property. This could affect subsequent tests.♻️ Suggested improvement
} finally { if (cryptoDescriptor) { Object.defineProperty(globalThis, 'crypto', cryptoDescriptor) + } else {+ Reflect.deleteProperty(globalThis, 'crypto') } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/ai-event-client/tests/runtime-id.test.ts` around lines 87 - 112, Update the test cleanup to restore globalThis.crypto correctly: after temporarily setting globalThis.crypto = undefined (using Object.defineProperty) ensure the finally block either restores the original descriptor when cryptoDescriptor is defined or deletes the temporary crypto property when cryptoDescriptor is undefined; reference the cryptoDescriptor variable and the call to getAIDevtoolsRuntimeId in this test so Math.random spy behavior remains isolated and subsequent tests don’t see a lingering undefined crypto.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/ai-event-client/tests/runtime-id.test.ts`:
- Around line 87-112: Update the test cleanup to restore globalThis.crypto
correctly: after temporarily setting globalThis.crypto = undefined (using
Object.defineProperty) ensure the finally block either restores the original
descriptor when cryptoDescriptor is defined or deletes the temporary crypto
property when cryptoDescriptor is undefined; reference the cryptoDescriptor
variable and the call to getAIDevtoolsRuntimeId in this test so Math.random spy
behavior remains isolated and subsequent tests don’t see a lingering undefined
crypto.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f0151164-0eb4-4e59-a104-021b4fb48a24
📒 Files selected for processing (3)
.changeset/lazy-runtime-id.mdpackages/ai-event-client/src/envelope.tspackages/ai-event-client/tests/runtime-id.test.ts
Uh oh!
There was an error while loading. Please reload this page.
🎯 Changes
@tanstack/ai-event-clientseeded its devtools runtime id at module scope (a top-level IIFE callingcrypto.randomUUID()/Math.random()). Because@tanstack/ai'schat()always pulls in the devtools middleware, this RNG ran at module-evaluation time — and Cloudflare Workers / edge runtimes forbid generating random values in global scope, so importingchat()crashed the Worker at startup/prerender (Disallowed operation called within global scope).Fix: generate the runtime id lazily on first use (in
getAIDevtoolsRuntimeId()/createAIDevtoolsEventEnvelope()) and memoize it, so module evaluation does no RNG. The cross-bundle global and generated id values are unchanged. Adds isolated regression tests asserting no RNG happens at import.Fixes#667.
✅ Checklist
pnpm run test:pr.🚀 Release Impact