chore(rivetkit): preemptively start manager/engine before serverless/runner has started

[NathanFlurry]

No description provided.

[NathanFlurry]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• chore(rivetkit): update RIVETKIT_* env var names to RIVET_* #3839
• doc: default to serverless api #3833
• chore(rivetkit): preemptively start manager/engine before serverless/runner has started #3834 👈 (View in Graphite)
• chore(rivetkit): update cloudflare workers and nextjs to latest #3803
• feat(dashboard): provide ns when querying kv #3831 : 1 other dependent PR (#3561 )
• chore: update sdks #3830
• refactor: performance improvements for inspector #3452
• fix(dashboard): use proper timestamp for actor status #3809
• fix(engine): require namespace for actor id-specific routes #3828
• 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.

[claude]

Code Review - PR #3834

Overview

This PR refactors the RivetKit startup lifecycle to preemptively initialize the manager/engine infrastructure before the deployment mode (serverless vs runner) is determined. This is a significant architectural improvement that addresses race conditions in serverless deployments.

---

✅ Strengths

Architecture

• Excellent separation of concerns: The two-phase startup (shared infrastructure → mode-specific config) is well-designed and clearly documented
• Idempotent operations: Both startServerless() and startRunner() are properly idempotent with guard clauses
• Smart auto-preparation: The setTimeout(..., 0) pattern in the Registry constructor is clever - it allows sync config modifications while still starting the runtime ASAP

Code Quality

• Clear documentation: The class-level comment in Runtime excellently explains the two-phase startup
• Better error handling: The duplicate invariant check removal (lines 107-111 in runtime/index.ts) eliminates redundancy
• Improved logging: The metadata.ts change to skip logging on first retry attempt reduces noise during normal startup

---

⚠️ Issues & Concerns

1. Race Condition in Registry Constructor (High Priority)

Location: rivetkit-typescript/packages/rivetkit/src/registry/index.ts:52-55

setTimeout(() => {
    // biome-ignore lint/nursery/noFloatingPromises: fire-and-forget auto-prepare
    this.#ensureRuntime();
}, 0);

Problem: This creates a race condition where:

• If registry.handler() is called synchronously (same tick), #ensureRuntime() runs twice
• The first call (from setTimeout) creates a promise
• The second call (from handler) sees #runtimePromise is undefined and creates another

Solution: The current implementation actually handles this correctly via #ensureRuntime()'s guard, but this is fragile. Consider:

constructor(config: RegistryConfigInput<A>) {
    this.#config = config;
    
    // Start runtime preparation on next tick to allow sync config modifications
    queueMicrotask(() => {
        // Safe to ignore - just warming up the runtime
        void this.#ensureRuntime();
    });
}

Using queueMicrotask is more semantically correct for this use case and signals intent better than setTimeout.

2. Async Handler Breaking Change (High Priority)

Location: rivetkit-typescript/packages/rivetkit/src/registry/index.ts:80-84

public async handler(request: Request): Promise<Response> {
    const runtime = await this.#ensureRuntime();
    runtime.startServerless();
    return await runtime.handleServerlessRequest(request);
}

Problem: Changing from Response | Promise<Response> to Promise<Response> is a breaking API change:

• Code expecting sync responses will break: const res = registry.handler(req); (res is now Promise)
• TypeScript will catch this, but it's still breaking

Impact: Users will need to add await everywhere they call handler(). Since this appears to be internal (based on the refactor), it might be acceptable, but should be documented.

Recommendation: Either:

1. Document this as a breaking change in release notes
2. Keep the return type as Response | Promise<Response> (though async functions always return Promise)

3. Unhandled Promise Rejection Risk (Medium Priority)

Location: rivetkit-typescript/packages/rivetkit/src/registry/index.ts:102-103

public startRunner() {
    // biome-ignore lint/nursery/noFloatingPromises: bg task
    this.#ensureRuntime().then((runtime) => runtime.startRunner());
}

Problem: If Runtime.create() fails (e.g., port already in use, engine download fails), the error is swallowed. This makes debugging very difficult.

Solution:

public startRunner() {
    // biome-ignore lint/nursery/noFloatingPromises: bg task
    this.#ensureRuntime()
        .then((runtime) => runtime.startRunner())
        .catch((err) => {
            logger().error({ msg: "failed to start runner", error: stringifyError(err) });
            // Re-throw to trigger unhandledRejection handler
            throw err;
        });
}

Same issue exists in:

• registry/index.ts:168 (legacy startNormal)
• registry/index.ts:54 (constructor setTimeout)

4. Missing Error Propagation (Medium Priority)

Location: rivetkit-typescript/packages/rivetkit/runtime/index.ts:176-177

if (this.#config.serverless.configureRunnerPool) {
    // biome-ignore lint/nursery/noFloatingPromises: intentional
    configureServerlessRunner(this.#config);
}

Problem: If configureServerlessRunner fails, the error is silently dropped. This could lead to a partially initialized system.

Solution: Either await it or add error handling:

if (this.#config.serverless.configureRunnerPool) {
    configureServerlessRunner(this.#config).catch((err) => {
        logger().error({ 
            msg: "failed to configure serverless runner", 
            error: stringifyError(err) 
        });
    });
}

5. Duplicate Invariant Check (Low Priority - Code Smell)

Location: rivetkit-typescript/packages/rivetkit/runtime/index.ts:107-111

The invariant at lines 107-111 duplicates the check at lines 85-88. While not harmful, it suggests the code at line 107 is in the wrong place or the invariant should be moved earlier.

Recommendation: Remove the duplicate check since it's already validated before entering the if block.

6. Type Safety: any Usage (Low Priority)

Location: rivetkit-typescript/packages/rivetkit/runtime/index.ts:122

let upgradeWebSocket: any;

Problem: Using any defeats TypeScript's type safety. The actual type should be determinable from crossPlatformServe's return type.

Solution: Type it properly based on the platform-specific WebSocket upgrade function.

---

🔒 Security Considerations

✅ No Critical Issues Found

• No SQL injection vectors (no DB queries in changed code)
• No XSS vectors (no HTML rendering)
• No command injection (engine process spawning uses proper API)
• No authentication bypasses

Minor Observation

The preemptive server start means port 6420 is bound earlier. Ensure:

• Proper firewall rules are in place
• The port binding happens with appropriate user permissions
• Error messages on port conflicts don't leak sensitive paths

---

🧪 Test Coverage

Missing Tests

This PR lacks tests for the new startup lifecycle. Recommended test cases:

1. Race condition test: Call handler() immediately after Registry creation
2. Idempotency test: Call startServerless() multiple times, verify single initialization
3. Mode conflict test: Call startServerless() then startRunner(), verify error
4. Async handler test: Verify handlers can await registry.handler()
5. Error propagation test: Mock Runtime.create() failure, verify error handling

Example test:

test("handler works when called immediately after construction", async () => {
    const registry = setup({ use: {} });
    const response = await registry.handler(new Request("http://localhost/test"));
    expect(response).toBeDefined();
});

---

🚀 Performance Considerations

✅ Improvements

• Faster cold starts: Preemptive manager/engine initialization means serverless platforms see reduced latency on first request
• Better resource utilization: Runtime is shared across all requests instead of per-request initialization

⚠️ Potential Concerns

• Memory usage: Runtime is now created eagerly on next tick. For short-lived processes, this wastes resources if never used
• Recommendation: Add a config option to disable auto-prepare for edge cases where it's not needed

---

📝 Documentation

CLAUDE.md Update

The comment guideline update is excellent - clear examples of what to avoid.

Missing Documentation

• No migration guide for the breaking async handler() change
• No documentation of when manager/engine starts vs. when serverless/runner configures
• The JSDoc comments are good but could mention the auto-prepare behavior

---

🎯 Recommendations Summary

Must Fix (Blocking)

1. Add proper error handling to all fire-and-forget promises
2. Document the handler() breaking change or maintain backward compatibility

Should Fix (High Priority)

1. Replace setTimeout(..., 0) with queueMicrotask()
2. Add test coverage for the new lifecycle
3. Remove duplicate invariant check

Nice to Have

1. Replace any with proper types
2. Add config option to disable auto-prepare
3. Add migration guide

---

✅ Approval Status

Approve with changes: The architecture is solid and this is a valuable improvement. The race condition handling and idempotency are well-designed. However, the unhandled promise rejections need to be addressed before merging to prevent difficult-to-debug failures in production.

Confidence Level

• Architecture: High ✅
• Correctness: Medium ⚠️ (promise rejection handling)
• Performance: High ✅
• Security: High ✅
• Test Coverage: Low ⚠️ (needs tests)

Great work on improving the startup lifecycle! The preemptive initialization is a smart solution to the serverless cold start problem.

[pkg-pr-new]

More templates

• actor-actions
• example-ai-agent
• example-ai-and-user-generated-actors-freestyle
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-cloudflare-workers-inline-client
• cross-actor-actions
• example-cursors
• example-cursors-raw-websocket
• custom-serverless
• example-sqlite
• example-elysia
• example-experimental-durable-streams-ai-agent
• hello-world
• example-hono
• hono-react
• example-kitchen-sink
• multi-region
• native-websockets
• next-js
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• scheduling
• state
• example-stream
• example-trpc

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/virtual-websocket

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 6eb42fb

[graphite-app]

Merge activity

• Jan 12, 10:01 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Jan 12, 10:02 PM UTC: CI is running for this pull request on a draft pull request (#3842) due to your merge queue CI optimization settings.
• Jan 12, 10:03 PM UTC: Merged by the Graphite merge queue via draft PR: #3842.