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/virtual-websocket

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

@rivetkit/cloudflare-workers

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

@rivetkit/db

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/db@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/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: 4e40c0b

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 critical Cloudflare Workers limitation by preventing unnecessary runtime preparation when it's not needed based on configuration. The change is well-targeted and solves a real platform constraint.

✅ Strengths

Correct Problem Identification: The fix properly addresses Cloudflare Workers' restriction on I/O operations outside the fetch context. The setTimeout(() => this.#ensureRuntime()) call would trigger Runtime.create() which calls ensureEngineProcess() - this performs file system operations that violate Cloudflare Workers constraints.
Minimal, Focused Change: The conditional check is precise and only guards the auto-prepare behavior, not breaking existing functionality.
Good Code Organization: Moving the Runtime import to be grouped with other external imports improves readability.

🔍 Code Quality Observations

Positive:

The logic correctly identifies when runtime preparation is needed (spawnEngine or serveManager)
The change is backward compatible - existing users won't see behavior changes

Suggestions for Improvement:

Potential Logic Gap (rivetkit-typescript/packages/rivetkit/src/registry/index.ts:51):
The condition checks config.serverless?.spawnEngine || config.serveManager, but there are other code paths that call #ensureRuntime():
- handler() method (line 83)
- startRunner() method (line 105)
- Legacy start() methods (lines 170)
If a Cloudflare Worker creates a Registry without spawnEngine or serveManager, then immediately calls registry.handler(request), the runtime will be created on-demand. However, the first call will be slower due to lazy initialization. Consider documenting this behavior or evaluating if handler() usage should also trigger auto-prepare (though this might reintroduce the Cloudflare issue).
Missing Test Coverage:
There don't appear to be tests verifying:
- Registry initialization doesn't trigger I/O when spawnEngine=false and serveManager=false
- Runtime is still properly initialized on first handler() call
- The optimization actually prevents the Cloudflare Workers error
Consider adding tests in rivetkit-typescript/packages/cloudflare-workers/tests/ that verify the runtime isn't prepared unnecessarily.

Documentation Gap:
The PR description mentions the optimization benefits, but the code doesn't document why this conditional exists. Consider adding a comment:

// Only auto-prepare runtime when we know it will be needed.
// This prevents I/O operations outside fetch context in serverless
// environments like Cloudflare Workers where file system access
// (used by ensureEngineProcess) is restricted.
if (config.serverless?.spawnEngine || config.serveManager) {

Type Safety:
The condition uses optional chaining (config.serverless?.spawnEngine) which is good, but relies on runtime config parsing. The TypeScript types don't enforce that these fields exist, which is fine given the schema validation, but worth noting.

🐛 Potential Issues

Low Severity:

Edge Case: If someone creates a Registry with both spawnEngine: false and serveManager: false, then never calls handler() or startRunner(), the runtime never gets created. This is probably fine (why create a registry you don't use?), but could lead to confusing errors if someone expects the runtime to exist.

🚀 Performance Considerations

Positive Impact:

Removes unnecessary async operation during Registry construction
Prevents downloading/spawning engine process when not needed
Reduces resource usage in pure client-only scenarios

Trade-off:

First request may be slightly slower in scenarios where runtime wasn't auto-prepared (lazy initialization overhead)

🔒 Security Considerations

No security concerns identified. The change reduces the attack surface by not spawning unnecessary processes.

📊 Test Coverage Recommendations

Consider adding tests for:

Cloudflare Workers Environment (in rivetkit-typescript/packages/cloudflare-workers/tests/):

it('should not auto-prepare runtime without spawnEngine or serveManager', () => {
  const registry = new Registry({
    use: { /* actors */ },
    serverless: { spawnEngine: false },
    serveManager: false
  });
  // Verify no I/O operations occurred
  // Verify runtime can still be created on-demand via handler()
});

Runtime Preparation (in rivetkit-typescript/packages/rivetkit/tests/):

it('should auto-prepare runtime when spawnEngine is true', async () => {
  const registry = new Registry({
    use: { /* actors */ },
    serverless: { spawnEngine: true }
  });
  await new Promise(resolve => setTimeout(resolve, 10));
  // Verify runtime was prepared
});

it('should auto-prepare runtime when serveManager is true', async () => {
  const registry = new Registry({
    use: { /* actors */ },
    serveManager: true
  });
  await new Promise(resolve => setTimeout(resolve, 10));
  // Verify runtime was prepared
});

✅ Final Recommendation

APPROVE with suggestions for follow-up improvements

The core fix is correct and necessary for Cloudflare Workers support. The suggested improvements (documentation, tests, edge case handling) can be addressed in follow-up PRs and don't block this change.

The PR successfully solves the immediate problem (RVT-5342) and follows the codebase conventions from CLAUDE.md.

Reviewed using Claude Code

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.

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

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

4e40c0b

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

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
jog1t wants to merge 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

Uh oh!

graphite-app bot commented Feb 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

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

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

Summary

✅ Strengths

🔍 Code Quality Observations

🐛 Potential Issues

🚀 Performance Considerations

🔒 Security Considerations

📊 Test Coverage Recommendations

✅ Final Recommendation

Uh oh!

graphite-app bot commented Feb 2, 2026

Graphite Automations

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Comments

jog1t commented Jan 26, 2026 •

edited

Loading

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

edited

Loading

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