fix(rivetkit): fix duplicate runner key in serverless

[NathanFlurry]

No description provided.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Nov 5, 2025 4:06am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 5, 2025 4:06am
rivet-inspector	Ignored	Preview		Nov 5, 2025 4:06am
rivet-site	Ignored	Preview		Nov 5, 2025 4:06am

[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(pegboard): allow configuring reschedule_backoff_max_exponent #3370
• fix(next-js): auto shut down runners when source code updates #3369
• fix(rivetkit): fix duplicate runner key in serverless #3366 👈 (View in Graphite)
• chore(runner): improve runner connection logs #3365
• chore(rivetkit): add runner sse ping interval #3362
• chore(rivetkit): log close code when client ws closes #3361
• chore(rivetkit): log runner id with actor logs #3360
• chore(scripts): add endian converter script #3359
• fix(guard): fix url decoding actor id + token #3357
• chore(rivetkit): remove unneeded headers & protocols in requests #3356
• fix(rivetkit): reduce pegboard actor retry for spawned engines #3355
• chore(pegboard): allow configuring base_retry_timeout, actor_start_threshold, actor_stop_threshold, and retry_reset_duration #3354
• feat(pb): add reschedule_ts to actors #3363
• fix: fix all dashboards #3358
• chore: change log levels #3333
• chore: misc bug fixes, add logs for outbound req #3332
• fix: use correct vbare functions for runner tunnel #3300 : 1 other dependent PR (#3301 )
• feat(grafana): fix api dashboards #3193
• fix: docker compose build #3273
• chore: fix remaining warnings for all targets #3305
• fix(gas): fix from_workflow when creating standalone ctx from op ctx #3342
• ci: udpate ci filter to succeed if no relevant changes #3343
• 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

This PR fixes duplicate runner key issues in serverless mode. The changes are well-targeted and solve the problem correctly. Here's my detailed review:

✅ Positive Aspects

1. Root Cause Fix: The PR correctly identifies and fixes the core issue:

   • Removed redundant fallback operators (??) in actor-driver.ts:112-114 that were incorrectly self-referencing (e.g., runConfig.namespace ?? runConfig.namespace)
   • Added proper fallback to crypto.randomUUID() for runnerKey when undefined

2. Config Schema Improvement: The change in config.ts is more appropriate:

   • Moved from .default() to .optional().transform() for runnerKey
   • This allows the value to remain undefined if not provided via config or env var
   • Cleaner separation of concerns - config layer sets from env, driver layer provides random fallback

3. Serverless-Specific Handling: The warning in router.ts:197-203 is good:

   • Explicitly warns when a user provides a runnerKey for serverless runners
   • Sets it to undefined so the driver layer generates a random one
   • Uses structured logging appropriately (?oldRunnerKey)

🔍 Observations

1. Similar Pattern in Line 95: In actor-driver.ts:95, there's still a redundant pattern:

   const token = runConfig.token ?? runConfig.token;

   This should probably be just const token = runConfig.token; (though it's harmless, it's redundant)

2. Typo in Schema Name: Minor typo at config.ts:5 - EngingConfigSchema should be EngineConfigSchema (though this appears to be pre-existing based on the import)

3. Consistency with Cloudflare: The pattern now aligns with the Cloudflare handler approach (see cloudflare-workers/src/handler.ts:39) which also sets runnerKey: "" as a placeholder, though the new approach is cleaner

🧪 Testing Considerations

Consider adding/verifying:

1. Test case for serverless runners receiving unique runner keys on each instantiation
2. Test case ensuring warning is logged when runnerKey is explicitly provided to serverless runners
3. Test case verifying RIVET_RUNNER_KEY env var is still respected in non-serverless mode

📝 Minor Suggestions

1. Line 95 cleanup: Remove the redundant fallback:

   const token = runConfig.token;

2. Consider adding a comment at actor-driver.ts:115 explaining why serverless needs random keys:

   // For serverless runners, generate a unique key per instance to avoid conflicts
   runnerKey: runConfig.runnerKey ?? crypto.randomUUID(),

✅ Overall Assessment

Approval: The core fix is solid and addresses the duplicate runner key issue properly. The serverless-specific handling is appropriate and well-documented through logging.

The changes follow TypeScript/Zod best practices and maintain backward compatibility while fixing the bug. The separation between config transformation and runtime fallback is cleaner than before.

Rating: Looks good to merge (pending the minor cleanup of line 95 if desired)

[claude]

Code Review

Summary

This PR fixes an issue where duplicate runner keys were being used in serverless environments. The fix ensures each serverless runner gets a unique random UUID, preventing runner conflicts.

Changes Analysis

1. File: actor-driver.ts (Lines 109-115)

• GOOD: Removed redundant self-assignments (runConfig.namespace ?? runConfig.namespace)
• GOOD: Moved runnerKey fallback logic to crypto.randomUUID() when not provided
• Clean and straightforward fix

2. File: config.ts (Lines 7-11)

• GOOD: Changed runnerKey from .default() to .optional().transform()
• GOOD: Now checks RIVET_RUNNER_KEY env variable before falling back to undefined
• This allows the caller to decide the fallback behavior instead of forcing a default UUID at config parse time

3. File: router.ts (Lines 189-195)

• GOOD: Added warning when user provides a runnerKey for serverless runners
• GOOD: Explicitly sets runnerKey to undefined so the actor driver generates a random one
• Clear logging helps with debugging

Strengths

1. Correct fix for the problem: The issue was that serverless runners would use the same runnerKey from config, causing conflicts. Now each serverless runner generates its own random key.

2. Good separation of concerns: The config layer no longer forces a default, allowing different execution contexts (serverless vs persistent) to handle keys appropriately.

3. Helpful logging: The warning in router.ts educates users about the behavior.

4. Consistent with Cloudflare pattern: I checked cloudflare-workers/src/handler.ts and it already sets runnerKey: "" as a workaround, noting that runner keys aren't used there. This PR makes the serverless pattern more explicit.

Suggestions

Minor: Code clarity

In actor-driver.ts:95, there's an existing code smell that should be addressed in a follow-up:

// Line 95 - redundant assignment
const token = runConfig.token ?? runConfig.token;

This appears to be leftover from a previous refactor (similar to the ones fixed in this PR). Should be:

const token = runConfig.token;

Minor: Type safety consideration

The transform() in config.ts changes the return type from string to string | undefined. Consider explicitly typing this or adding a code comment explaining why runnerKey can be undefined here (because different execution contexts provide it differently).

runnerKey: z
    .string()
    .optional()
    .transform((x) => x ?? getEnvUniversal("RIVET_RUNNER_KEY")), // Returns string | undefined - callers must provide fallback

Testing: Missing test coverage

There are no tests verifying the serverless runner key behavior. Consider adding a test that:

1. Verifies serverless runners ignore user-provided runnerKey
2. Verifies each serverless runner gets a unique key
3. Verifies the warning is logged when a user provides a runnerKey

Example test location: Create a test in the driver test suite or add to router tests.

Potential Issues

None identified - The changes are safe and well-contained.

Security Considerations

No concerns - Using crypto.randomUUID() is appropriate for generating unique runner identifiers.

Performance Considerations

No impact - UUID generation is negligible overhead during runner initialization.

Verdict

✅ LGTM with minor suggestions

The fix correctly addresses the duplicate runner key issue in serverless environments. The code changes are clean, logical, and maintain backward compatibility. The suggestions above are minor improvements that could be addressed in follow-up PRs.

Great work on identifying and fixing this subtle but important bug!

[claude]

Code Review

Summary

This PR fixes an issue where duplicate runner keys were being used in serverless mode, ensuring that serverless runners always get unique runner keys. The changes are focused and address the specific issue.

Positive Observations

1. Good Fix: The core fix properly addresses the duplicate runner key issue by:

   • Removing the redundant fallback operators in actor-driver.ts (lines 112-115)
   • Ensuring runnerKey defaults to a random UUID when not provided (line 115)
   • Explicitly setting runnerKey to undefined in serverless mode (router.ts:194) so the random UUID fallback is triggered

2. Helpful Warning: The warning log added in router.ts:190-193 is useful for developers who might be confused why their configured runner key is being ignored in serverless mode.

3. Cleaner Schema: The config schema change (config.ts:10-11) using .transform() is cleaner than the previous .default() approach with an arrow function.

Issues & Concerns

1. Redundant Code Removed (Minor Fix Needed)

In actor-driver.ts:95, there's still a duplicate assignment pattern:

const token = runConfig.token ?? runConfig.token;

This should just be:

const token = runConfig.token;

2. Schema Behavior Change (Potential Breaking Change)

The config schema change has a subtle but important behavior difference:

Before:

runnerKey: z.string().default(() => getEnvUniversal("RIVET_RUNNER_KEY") ?? crypto.randomUUID())

• Missing/undefined values → check env var → fallback to random UUID
• Empty string ("") → accepted as-is

After:

runnerKey: z.string().optional().transform((x) => x ?? getEnvUniversal("RIVET_RUNNER_KEY"))

• Missing/undefined values → check env var → might be undefined
• Empty string ("") → check env var (because "" ?? envVar evaluates to envVar)

Issues:

• Empty strings now trigger the env var check (probably unintended)
• If env var is also not set, runnerKey could be undefined at the schema level
• The actual UUID fallback now happens in actor-driver.ts:115, creating two-stage defaulting logic

Recommendation: Consider using .transform() with explicit empty string handling:

runnerKey: z.string()
  .optional()
  .transform((x) => x || getEnvUniversal("RIVET_RUNNER_KEY") || undefined)

Or document that the final UUID fallback intentionally happens in the driver constructor.

3. Type Safety Concern

After the schema change, runConfig.runnerKey could be undefined, but the type system might not reflect this everywhere. The driver handles this correctly at line 115, but it would be good to verify that all consumers of RunnerConfig.runnerKey handle the optional case.

4. Missing Test Coverage

There don't appear to be any tests verifying:

• Serverless runners get unique runner keys
• The warning is logged when a runner key is provided in serverless mode
• Runner keys work correctly in non-serverless mode with env vars

Consider adding tests to prevent regression.

Minor Observations

1. Typo in Schema Name: EngingConfigSchema should probably be EngineConfigSchema (missing 'e'), but this is pre-existing and outside the scope of this PR.

2. Documentation: The warning message is clear, but it might be worth adding a comment in the code explaining why runner keys aren't supported in serverless mode.

Performance & Security

✅ No performance concerns
✅ No security issues
✅ Random UUIDs provide good uniqueness guarantees for runner keys

Recommendation

Approve with minor fixes suggested. The core logic is sound, but please:

1. Fix the redundant token assignment at line 95
2. Consider the schema behavior change and document or adjust as needed
3. Add test coverage for this functionality

---

📝 Reviewed with attention to the project's CLAUDE.md conventions

[claude]

Code Review

This PR fixes an issue where duplicate runner keys were being used in serverless mode. The changes are well-targeted and solve the problem correctly.

✅ Strengths

1. Root Cause Fix: Correctly identifies and fixes the core issue:

   • Removed redundant self-referencing fallback operators in actor-driver.ts:112-114 (e.g., runConfig.namespace ?? runConfig.namespace)
   • Added proper fallback to crypto.randomUUID() for runnerKey when undefined

2. Config Schema Improvement: The change in config.ts is more appropriate:

   • Moved from .default() to .optional().transform() for runnerKey
   • Allows the value to remain undefined if not provided via config or env var
   • Cleaner separation of concerns - config layer reads from env, driver layer provides random fallback

3. Serverless-Specific Handling: Good warning in router.ts:189-195:

   • Explicitly warns when user provides a runnerKey for serverless runners
   • Sets it to undefined so the driver layer generates a random one
   • Uses structured logging appropriately

🔍 Issues Found

1. Redundant Code Still Present

Location: actor-driver.ts:95

Issue: Similar redundant pattern still exists:

const token = runConfig.token ?? runConfig.token;

Fix: Should be:

const token = runConfig.token;

2. Schema Behavior Change

Location: config.ts:10-11

Issue: The .transform() behavior differs from the original .default():

• Empty strings ("") now trigger the env var fallback (because "" ?? envVar evaluates to envVar)
• This might be unintended behavior

Recommendation: Consider explicit handling:

.transform((x) => (x && x.length > 0) ? x : getEnvUniversal("RIVET_RUNNER_KEY"))

Or use .or() if empty strings should be treated as missing values.

🧪 Test Coverage Concerns

No tests found verifying:

1. Serverless runners receive unique runner keys on each instantiation
2. Warning is logged when runnerKey is explicitly provided to serverless runners
3. RIVET_RUNNER_KEY env var is still respected in non-serverless mode
4. Multiple serverless instances don't conflict with duplicate keys

Recommendation: Add test coverage for this functionality to prevent regression.

📝 Minor Suggestions

1. Add explanatory comment at actor-driver.ts:115:

   // Serverless runners need unique keys per instance to avoid conflicts
   runnerKey: runConfig.runnerKey ?? crypto.randomUUID(),

2. Type safety verification: Ensure all consumers of runConfig.runnerKey properly handle it being undefined (though the driver fallback at line 115 should catch this).

✅ Overall Assessment

Verdict: Approve with minor fixes recommended

The core fix is solid and properly addresses the duplicate runner key issue in serverless environments. The separation between config transformation and runtime fallback is cleaner than before. Please address the redundant code at line 95 and consider adding test coverage.

Security: ✅ No concerns - crypto.randomUUID() is appropriate
Performance: ✅ No impact - UUID generation overhead is negligible
Code Quality: ✅ Good - follows structured logging patterns per CLAUDE.md

[claude]

Code Review

This PR fixes duplicate runner key issues in serverless mode and cleans up redundant fallback logic. Overall the changes look good, but I have some feedback on code quality and potential improvements.

---

Positive Changes

1. Fixed redundant fallback logic - Good catch removing the duplicated ?? operators that were comparing the same value to itself (e.g., runConfig.token ?? runConfig.token)

2. Proper serverless runner key handling - The warning message in router.ts:189-195 is helpful for developers who might not realize that runner keys aren't supported in serverless mode

3. Config schema improvement - Moving runnerKey to use optional().transform() instead of default() is cleaner and allows proper environment variable fallback

---

Issues & Suggestions

1. Code Quality: Logging Pattern (rivetkit-typescript/packages/rivetkit/src/manager/router.ts:190-193)

The current logging doesn't follow the structured logging pattern mentioned in the codebase conventions. The log message should be lowercase and parameters should be structured separately:

// Current
logger().warn({
    msg: "runner keys are not supported by serverless runners, this will be overwritten with a random runner key",
    oldRunnerKey: newRunConfig.runnerKey,
});

// Suggested
logger().warn({
    oldRunnerKey: newRunConfig.runnerKey,
    msg: "runner keys are not supported by serverless runners, will be overwritten with random key"
});

Per CLAUDE.md: "Log messages should be lowercase unless mentioning specific code symbols."

2. Potential Bug: Environment Variable Priority (rivetkit-typescript/packages/rivetkit/src/drivers/engine/config.ts:8-11)

The new transform logic may have unexpected behavior:

runnerKey: z
    .string()
    .optional()
    .transform((x) => x ?? getEnvUniversal("RIVET_RUNNER_KEY")),

Issue: This will return undefined if both the input x and the environment variable are not set. Then in actor-driver.ts:115, it falls back to crypto.randomUUID(). This works but creates an implicit dependency chain.

Consideration: Should the environment variable be checked before the transform, or is the current behavior intentional? The old code had the UUID generation in the schema itself.

3. Missing Test Coverage

I notice there are no tests covering:

• Serverless runner initialization with/without runnerKey
• The warning when runnerKey is set for serverless runners
• Environment variable RIVET_RUNNER_KEY fallback behavior

Recommendation: Add tests to verify:

1. Serverless runners generate random UUIDs when no key is provided
2. Warning is logged when user sets a runnerKey for serverless
3. Environment variable RIVET_RUNNER_KEY is properly used when set

4. Consistency with Cloudflare Workers

I notice that packages/cloudflare-workers/src/handler.ts uses an empty string for runnerKey:

inputConfig = { ...inputConfig, runnerKey: "" };

Question: Should serverless runners use the same pattern (empty string) instead of undefined for consistency? Or is there a reason for the different approaches?

---

Security Considerations

No security concerns identified. The use of crypto.randomUUID() for generating runner keys is appropriate.

---

Performance Considerations

The changes have minimal performance impact. The only addition is the warning log for serverless runners, which is appropriate.

---

Summary

Approve with minor suggestions. The core fix is solid and addresses the duplicate runner key issue. The main recommendations are:

1. Update log message to follow structured logging conventions
2. Consider adding test coverage for serverless runner key behavior
3. Clarify if the environment variable fallback behavior is intentional
4. Consider consistency with Cloudflare Workers pattern

The PR can be merged after addressing the logging pattern issue.

[claude]

PR Review: fix(rivetkit): fix duplicate runner key in serverless

Summary

This PR fixes issues with the runnerKey configuration in serverless mode by:

1. Removing redundant fallback logic that was using the same value twice
2. Moving the default random UUID generation from the config schema to the actual usage point
3. Adding explicit warning and override for serverless runners

Code Quality ✅

Positive Changes:

• Removed redundant code: Lines like runConfig.token ?? runConfig.token and runConfig.namespace ?? runConfig.namespace were correctly identified as no-ops
• Clear separation of concerns: The config schema now only handles environment variable transformation, while the driver handles the actual default generation
• Good logging: The warning message in router.ts:190-195 is helpful and uses structured logging correctly

Potential Issues 🔍

1. Breaking Behavior Change (Minor)
The runnerKey config behavior has changed:

• Before: Default value was generated at config parse time (either from RIVET_RUNNER_KEY env var or random UUID)
• After: Config transform returns undefined if no env var exists, UUID generation happens later in actor-driver.ts:115

This should be fine, but the logic could be clearer. Currently:

• config.ts:11 transforms to env var OR undefined
• actor-driver.ts:115 generates UUID if still undefined
• router.ts:194 explicitly sets to undefined for serverless

2. Type Safety Consideration
In config.ts:8-11, the runnerKey is marked as .optional() with a transform. This means the type allows undefined, but the transform function (x) => x ?? getEnvUniversal("RIVET_RUNNER_KEY") could still return undefined if the env var doesn't exist. The code correctly handles this, but consider documenting this behavior.

3. Serverless Override Logic
In router.ts:189-195, the code warns if runnerKey is set and then overrides it to undefined. This is good, but since actor-driver.ts:115 will then generate a random UUID anyway, consider whether the warning message should mention that a random key will be generated:

logger().warn({
    msg: "runner keys are not supported by serverless runners, using a random runner key instead",
    oldRunnerKey: newRunConfig.runnerKey,
});

Performance ✅

No performance concerns - the changes are minimal and don't introduce any new expensive operations.

Security ✅

No security concerns. The change from predictable environment-based keys to random UUIDs for serverless runners actually improves isolation.

Test Coverage ⚠️

Missing Tests:
While the fix is straightforward, consider adding tests for:

1. Serverless runner behavior with an explicit runnerKey (should warn and override)
2. Serverless runner behavior without runnerKey (should generate random UUID)
3. Non-serverless runner behavior (should respect config)

The existing test files (driver-engine.test.ts, etc.) would be good places to add these.

Recommendations

1. Minor: Update the warning message in router.ts:191 to mention that a random key will be used
2. Nice to have: Add a comment in config.ts:7-11 explaining that undefined is expected when no env var is set, and the actual UUID generation happens in the driver
3. Nice to have: Add test coverage for the serverless runner key behavior

Verdict: ✅ Approve with Minor Suggestions

The PR successfully fixes the duplicate runner key issue and removes redundant code. The changes are clean and follow the codebase conventions (structured logging, kebab-case error messages). The suggestions above are minor improvements that could be addressed in a follow-up if needed.

Great work fixing those redundant null coalescing operators!

[claude]

PR Review: Fix duplicate runner key in serverless

Summary

This PR fixes a bug where redundant null-coalescing operators (??) were being used, and ensures that serverless runners always use random runner keys instead of potentially reusing configured keys.

Code Quality ✅

Positive Changes:

1. Removed redundant code - Lines like runConfig.token ?? runConfig.token were correctly simplified to just runConfig.token
2. Clear intent - The changes make it explicit that serverless runners don't support custom runner keys
3. Good logging - Added a helpful warning when a user tries to configure a runner key for serverless mode (router.ts:190-195)

Code Style:

• Follows structured logging patterns correctly using logger().warn({msg: "...", oldRunnerKey: ...})
• Message is properly lowercase per project conventions

Logic & Implementation ✅

config.ts changes:

• Changed from .default(() => getEnvUniversal("RIVET_RUNNER_KEY") ?? crypto.randomUUID()) to .optional().transform((x) => x ?? getEnvUniversal("RIVET_RUNNER_KEY"))
• This is cleaner and moves the random UUID generation responsibility to the actor driver

actor-driver.ts changes:

• Line 122: runnerKey: runConfig.runnerKey ?? crypto.randomUUID() - Good fallback behavior
• Removed redundant null-coalescing on lines 119-121 (namespace, totalSlots, runnerName)

router.ts changes:

• Lines 189-195: Explicitly clears runnerKey for serverless runners with appropriate warning
• This prevents key reuse across serverless invocations, which is correct behavior

Potential Concerns ⚠️

1. Breaking behavior change: The config schema change from .default() to .optional().transform() could be a subtle breaking change:

   • Before: runnerKey would always have a value (env var or random UUID)
   • After: runnerKey could be undefined if no env var is set
   • This is mitigated by the fallback in actor-driver.ts:122, but it changes when the UUID is generated

2. Timing consideration: Moving UUID generation from config parse time to driver construction time means:

   • Multiple drivers created from the same config will now get different UUIDs (probably desired)
   • But this behavior change isn't explicitly documented

Security ✅

No security concerns. Using crypto.randomUUID() is appropriate for generating unique runner keys.

Performance ✅

No performance concerns. The changes are minimal and don't affect hot paths.

Test Coverage ❓

Missing test coverage:

• No tests verify that serverless runners get random runner keys
• No tests verify the warning is logged when runnerKey is set for serverless
• No tests verify that the fallback to crypto.randomUUID() works correctly

Recommendation: Consider adding a test that:

// Verify serverless runner gets random key even if configured
test('serverless runner ignores configured runnerKey', async () => {
  const config = { runnerKey: 'my-key' };
  // ... create serverless runner
  // Assert warning was logged
  // Assert runner actually uses a different random key
});

Documentation 📝

The warning message in router.ts:190-192 serves as inline documentation, but consider:

• Adding a comment in config.ts explaining why runnerKey is optional
• Documenting the serverless behavior difference in a comment

Minor Issues

1. Typo in schema name: EngingConfigSchema should be EngineConfigSchema (line 5 of config.ts) - though this is pre-existing
2. Comment consistency: The comment on line 100-101 of actor-driver.ts mentions "HACK" but the fix on line 102 makes it not redundant anymore - consider updating the comment

Verdict: ✅ Approve with minor suggestions

The core fix is correct and addresses the issue. The changes are logical and improve code quality by removing redundant operations. The main gap is test coverage, but the implementation itself is sound.

Suggested follow-ups (non-blocking):

• Add test coverage for serverless runner key behavior
• Fix the EngingConfigSchema typo in a separate PR
• Update the HACK comment in actor-driver.ts since the code is now correct

---

🤖 Generated with Claude Code

[graphite-app]

Merge activity

• Nov 5, 5:07 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Nov 5, 5:08 AM UTC: CI is running for this pull request on a draft pull request (#3371) due to your merge queue CI optimization settings.
• Nov 5, 5:11 AM UTC: Merged by the Graphite merge queue via draft PR: #3371.