fix(cloudflare-workers): do not use i/o operations outside fetch context by jog1t · Pull Request #4042 · rivet-dev/rivet

jog1t · 2026-01-26T20:47:36Z

Closes RVT-5342

TL;DR

Only auto-prepare the runtime when necessary based on configuration.

What changed?

Added a conditional check before auto-preparing the runtime in the Registry constructor. Now, the runtime is only auto-prepared when either config.serverless?.spawnEngine or config.serveManager is set to a truthy value.

Also reordered the imports to place the Runtime import with the other imports from external files.

How to test?

Create a Registry instance with serverless.spawnEngine set to false and serveManager set to false
Verify that the runtime is not automatically prepared
Create a Registry instance with either serverless.spawnEngine or serveManager set to true
Verify that the runtime is automatically prepared

Why make this change?

This optimization prevents unnecessary runtime preparation when it's not needed based on the configuration. This can improve performance and reduce resource usage in scenarios where the runtime isn't required.

jog1t · 2026-01-26T20:47:52Z

fix(cloudflare-workers): do not use i/o operations outside fetch context #4042 👈 (View in Graphite)
main

How to use the Graphite Merge Queue

Add the label merge-queue to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

_{An organization admin has enabled the Graphite Merge Queue in this repository.} _{Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.}

This stack of pull requests is managed by Graphite. Learn more about stacking.

linear · 2026-01-26T20:48:24Z

RVT-5342 # Bug: "Disallowed operation called within global scope" error in v2.0.41

pkg-pr-new · 2026-01-26T20:49:59Z

More templates

@rivetkit/cloudflare-workers

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/cloudflare-workers@4042

@rivetkit/framework-base

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/framework-base@4042

@rivetkit/next-js

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/next-js@4042

@rivetkit/react

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/react@4042

rivetkit

pnpm add https://pkg.pr.new/rivet-dev/rivet/rivetkit@4042

@rivetkit/sql-loader

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sql-loader@4042

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4042

@rivetkit/traces

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/traces@4042

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4042

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4042

@rivetkit/engine-runner

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner@4042

@rivetkit/engine-runner-protocol

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/engine-runner-protocol@4042

commit: f21521a

claude · 2026-01-26T20:50:19Z

Code Review - PR #4042

Summary

This PR addresses a Cloudflare Workers-specific issue by conditionally preparing the runtime only when necessary. The change prevents I/O operations from running outside of the fetch context, which is prohibited in Cloudflare Workers.

✅ Positive Aspects

Correct Problem Identification: The fix appropriately addresses the Cloudflare Workers constraint where I/O operations cannot occur outside the request/fetch context.
Minimal, Targeted Change: The change is surgical and focused on the specific issue without over-engineering.
Import Organization: The reordering of the Runtime import to group it with other external imports improves code organization.

🔍 Issues & Concerns

1. Logic Error - Missing `serveManager` Check 🔴 Critical

The conditional check on line 51 has a logical flaw:

if (config.serverless?.spawnEngine || config.serveManager) {

Problem: Looking at runtime/index.ts:77-149, the Runtime.create() method shows that runtime preparation involves I/O operations when either:

config.serverless.spawnEngine is true (downloads/spawns engine process)
config.serveManager is true (starts manager server)

However, your condition will auto-prepare even when only serveManager is true. This defeats the purpose of the fix because:

In Cloudflare Workers, serveManager should typically be false since:

Cloudflare Workers uses the CloudflareActorsManagerDriver instead of the built-in manager
The Cloudflare handler (packages/cloudflare-workers/src/handler.ts:73-80) explicitly disables many features and doesn't serve a manager
Auto-preparing when serveManager=true would still trigger I/O operations outside fetch context

Recommendation: The condition should be more precise. Consider:

// Only auto-prepare when we know we need runtime infrastructure upfront
// For serverless platforms like Cloudflare Workers, preparation happens lazily on first request
if (config.serverless?.spawnEngine) {
  // Auto-prepare on next tick (gives time for sync config modification)
  setTimeout(() => {
    // biome-ignore lint/nursery/noFloatingPromises: fire-and-forget auto-prepare
    this.#ensureRuntime();
  }, 0);
}

Or if you need both conditions:

// Auto-prepare when serving local infrastructure (not for pure serverless like CF Workers)
if (config.serverless?.spawnEngine || (config.serveManager && \!config.serverless?.configureRunnerPool)) {

2. Incomplete Testing Guidance 🟡 Medium

The "How to test?" section in the PR description doesn't address the actual Cloudflare Workers use case:

Current test plan:

Create a Registry instance with serverless.spawnEngine set to false and serveManager set to false

Missing:

How to test this specifically fixes the Cloudflare Workers fetch context issue?
What error was occurring before? (Should document the original error message)
A test case that actually deploys to Cloudflare Workers or uses their local dev environment

Recommendation: Add specific Cloudflare Workers test steps:

### Cloudflare Workers Test:
1. Deploy a Registry to Cloudflare Workers with default config
2. Verify no I/O errors occur during module initialization
3. Verify the first request successfully initializes the runtime
4. Verify subsequent requests use the cached runtime

3. No Test Coverage 🟡 Medium

There are no automated tests added to verify this behavior. Given that this is a platform-specific fix, consider:

Adding a test in packages/cloudflare-workers/tests/ that verifies runtime is not auto-prepared
Adding a test that verifies lazy initialization works correctly on first request

4. Import Reordering 🟢 Low Priority (Style)

While the import reordering is good practice, it's unrelated to the main fix. In projects with strict PR practices, this would typically be a separate commit to keep changes focused. Not a blocker, just a note for future PRs.

5. Potential Edge Case 🟡 Medium

Question: What happens if someone calls registry.startRunner() or registry.handler() before the runtime is prepared?

Looking at the code:

Line 103: startRunner() calls this.#ensureRuntime() ✅ Safe
Line 82-85: handler() calls this.#ensureRuntime() ✅ Safe

This is fine - the lazy initialization pattern is correctly implemented. However, it might be worth adding a comment explaining this pattern for maintainability.

📝 Suggested Improvements

Fix the conditional logic (see Issue [SVC-2555] Set up issue templates #1 above)
Add JSDoc comment explaining the lazy initialization strategy:

/**
 * Registry for Rivet Actors.
 *
 * Runtime initialization is deferred to support serverless platforms that
 * prohibit I/O during module initialization (e.g., Cloudflare Workers).
 * The runtime auto-prepares on platforms that need it (spawnEngine),
 * otherwise initialization happens lazily on first handler/startRunner call.
 */
export class Registry<A extends RegistryActors> {

Add tests for the Cloudflare Workers package verifying this behavior
Document the original error in the commit message or PR for future reference

🎯 Recommendation

Request Changes - The logic in the conditional check needs to be corrected before merging. The current implementation may still cause issues in certain configurations.

Files Changed: 1 file, +8/-6 lines
Risk Level: Low-Medium (single file, but critical path for initialization)
Deployment Impact: Affects all Cloudflare Workers deployments

claude · 2026-01-30T21:31:36Z

PR Review: fix(cloudflare-workers): do not use i/o operations outside fetch context

Summary

This PR addresses a Cloudflare Workers limitation by preventing unnecessary runtime preparation when it's not needed. The change is well-targeted and the import reorganization is a nice cleanup.

Bug: Dev Mode Regression

The condition uses the raw input config object rather than the parsed config with defaults applied:

if (config.serverless?.spawnEngine || config.serveManager) {

From registry/config/index.ts:233:

// Determine serveManager: default to true in dev mode without endpoint, false otherwise
const serveManager = config.serveManager ?? (isDevEnv && \!endpoint);

In dev mode (NODE_ENV=development), serveManager defaults to true after schema parsing — but the raw input config.serveManager remains undefined (falsy). So the auto-prepare setTimeout will silently skip in dev mode unless the user explicitly sets serveManager: true, which is a regression from the previous getNodeEnv() \!== 'test' behavior.

Cloudflare Workers is unaffected because isDevEnv is false in that environment, so serveManager would default to false there anyway.

Suggested fix — move the condition inside the setTimeout callback where the parsed config can be used safely (after any sync config modifications):

// Auto-prepare on next tick (gives time for sync config modification)
setTimeout(() => {
    const parsedConfig = this.parseConfig();
    if (parsedConfig.serverless?.spawnEngine || parsedConfig.serveManager) {
        // biome-ignore lint/nursery/noFloatingPromises: fire-and-forget auto-prepare
        this.#ensureRuntime();
    }
}, 0);

This handles the Cloudflare Workers case (no I/O outside fetch context when neither flag is set), preserves dev mode behavior (auto-prepares when serveManager defaults to true), and correctly handles explicit configuration.

Minor: Comment placement

The comment moved inside the conditional, which is fine, but it describes the setTimeout mechanic rather than explaining why the condition exists. Consider adding context about the Cloudflare Workers restriction:

// Only auto-prepare when a local process will be needed. This avoids
// I/O operations outside the fetch context in environments like Cloudflare
// Workers where file system access is restricted.
if (...) {
    // Deferred to next tick to allow sync config modifications to complete.
    setTimeout(() => { ... }, 0);
}

Test Coverage

The PR description has a good manual test plan, but an automated regression test would help catch this going forward — e.g., a test that verifies:

Registry in dev mode without explicit serveManager triggers auto-prepare
Registry in a "Cloudflare-like" config (no spawnEngine, no serveManager) does not trigger auto-prepare outside a fetch context

graphite-app · 2026-02-02T21:55:14Z

Graphite Automations

"Test" took an action on this PR • (02/02/26)

1 assignee was added to this PR based on Kacper Wojciechowski's automation.

NathanFlurry · 2026-03-03T22:26:20Z

This change is part of the following stack:

fix(cloudflare-workers): do not use i/o operations outside fetch context #4042 ◀

_{Change managed by git-spice.}

jog1t requested a review from NathanFlurry January 26, 2026 20:48

jog1t force-pushed the 01-26-fix_cloudflare-workers_do_not_use_i_o_operations_outside_fetch_context branch from d987869 to 4e40c0b Compare January 30, 2026 21:28

jog1t marked this pull request as ready for review February 2, 2026 21:51

graphite-app bot assigned jog1t Feb 2, 2026

fix(cloudflare-workers): do not use i/o operations outside fetch context

7fb6538

NathanFlurry force-pushed the 01-26-fix_cloudflare-workers_do_not_use_i_o_operations_outside_fetch_context branch from 4e40c0b to 7fb6538 Compare March 3, 2026 22:24

NathanFlurry merged commit 072ce3e into main Mar 3, 2026
7 of 17 checks passed

NathanFlurry deleted the 01-26-fix_cloudflare-workers_do_not_use_i_o_operations_outside_fetch_context branch March 3, 2026 22:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cloudflare-workers): do not use i/o operations outside fetch context#4042

fix(cloudflare-workers): do not use i/o operations outside fetch context#4042
NathanFlurry merged 1 commit intomainfrom
01-26-fix_cloudflare-workers_do_not_use_i_o_operations_outside_fetch_context

jog1t commented Jan 26, 2026 •

edited

Loading

Uh oh!

jog1t commented Jan 26, 2026

Uh oh!

linear bot commented Jan 26, 2026

Uh oh!

pkg-pr-new bot commented Jan 26, 2026 •

edited

Loading

Uh oh!

claude bot commented Jan 26, 2026

Uh oh!

claude bot commented Jan 30, 2026 •

edited

Loading

Uh oh!

graphite-app bot commented Feb 2, 2026

Uh oh!

NathanFlurry commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

jog1t commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TL;DR

What changed?

How to test?

Why make this change?

Uh oh!

jog1t commented Jan 26, 2026

How to use the Graphite Merge Queue

Uh oh!

linear bot commented Jan 26, 2026

Uh oh!

pkg-pr-new bot commented Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Jan 26, 2026

Code Review - PR #4042

Summary

✅ Positive Aspects

🔍 Issues & Concerns

1. Logic Error - Missing serveManager Check 🔴 Critical

2. Incomplete Testing Guidance 🟡 Medium

3. No Test Coverage 🟡 Medium

4. Import Reordering 🟢 Low Priority (Style)

5. Potential Edge Case 🟡 Medium

📝 Suggested Improvements

🎯 Recommendation

Uh oh!

claude bot commented Jan 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: fix(cloudflare-workers): do not use i/o operations outside fetch context

Summary

Bug: Dev Mode Regression

Minor: Comment placement

Test Coverage

Uh oh!

graphite-app bot commented Feb 2, 2026

Graphite Automations

Uh oh!

NathanFlurry commented Mar 3, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jog1t commented Jan 26, 2026 •

edited

Loading

pkg-pr-new bot commented Jan 26, 2026 •

edited

Loading

1. Logic Error - Missing `serveManager` Check 🔴 Critical

claude bot commented Jan 30, 2026 •

edited

Loading