chore: fix serverless config #1361

MasterPtato · 2025-10-06T23:53:42Z

No description provided.

vercel · 2025-10-06T23:53:48Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Oct 7, 2025 9:20pm

claude · 2025-10-06T23:53:59Z

Claude finished @MasterPtato's task —— View job

PR Review: chore: fix serverless config

Todo List:

Read repository guidelines (CLAUDE.md)
Analyze changes in examples/counter-serverless/scripts/connect.ts
Analyze changes in packages/next-js/src/mod.ts
Analyze changes in packages/rivetkit/src/registry/mod.ts
Provide comprehensive review feedback
Complete review

Review Feedback

I've analyzed all three files and their changes. Overall, these are reasonable configuration fixes for serverless environments. Here's my detailed feedback:

✅ Code Quality & Best Practices

Positive aspects:

Changes follow consistent patterns and are well-scoped
No unnecessary complexity introduced
Proper use of fallback values with ?? operator

Areas for improvement:

Missing PR description: Consider adding a description explaining the rationale for these configuration changes
Magic numbers: Some hardcoded values could benefit from named constants

⚠️ Potential Issues

examples/counter-serverless/scripts/connect.ts:7-19
- Commented out event handling: The .connect() call and event listener are commented out, which might break event-driven functionality in this example
- Commented out disposal: The counter.dispose() call is commented out, which could lead to resource leaks in production scenarios
- Recommendation: Either remove these lines entirely or add comments explaining why they're disabled
packages/rivetkit/src/registry/mod.ts:241-248
- Breaking change: min_runners changed from 0 to 1 - this could impact cost optimization for low-traffic serverless deployments
- Time unit inconsistency: request_lifespan changed from milliseconds (15 * 60_000) to seconds (15 * 60) - ensure this aligns with the API expectations
- Slot reduction: slots_per_runner reduced from 10000 to 1000 - this could impact throughput capacity

🔧 Configuration Concerns

packages/next-js/src/mod.ts:14

Port fallback change: Changing from 8080 to 3000 aligns better with Next.js defaults, which is good for developer experience

packages/rivetkit/src/registry/mod.ts

Resource allocation: The slot reduction might be intentional for memory optimization, but should be documented
Cost implications: Setting min_runners to 1 instead of 0 means always having at least one runner active

🚀 Performance Considerations

Positive: Reduced slots per runner (1000 vs 10000) may improve memory usage and startup time
Concern: Reduced slots could create bottlenecks under high load - ensure this aligns with expected traffic patterns

🔒 Security

No security concerns identified in these changes.

🧪 Test Coverage

Missing: No test updates accompanying these configuration changes. Consider adding tests that verify:

Serverless configuration behavior with new defaults
Example script functionality with commented-out sections
Resource allocation limits work as expected

📝 Recommendations

Add TODO resolution: The // TODO: 0 comment suggests min_runners should eventually be 0 - consider creating an issue to track this
Documentation: Update any relevant documentation about default resource allocation
Verification: Test the example script to ensure it still demonstrates the intended functionality
Rollback plan: Document the rationale for these specific values in case rollback is needed

Summary

This PR makes sensible configuration adjustments for serverless environments, but some changes (especially resource limits and commented-out functionality) should be validated against intended behavior and documented for future reference.

MasterPtato · 2025-10-06T23:54:01Z

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(core): handle errors inside message handlers #1364
chore(core): add smoke test #1363
chore: fix serverless config #1361 👈 (View in Graphite)
chore(core): add x-rivet-endpoint header for serverless runners & cleanup severless runner setup #1359
fix(next): fix local dev #1352
chore(core): add support for auto-configuring serverless #1356 : 1 other dependent PR (#1357 )
chore(core): add build:fast script #1355
feat(core): support running engine #1353
fix(example-deno): fix check-types #1351
chore(core): simplify config #1350
main

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

merge queue - adds this PR to the back of the merge queue
hotfix - for urgent hot fixes, skip the queue and merge this PR next

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.

pkg-pr-new · 2025-10-06T23:56:19Z

More templates

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

commit: 4d2265c

claude · 2025-10-07T17:53:02Z

Claude finished @MasterPtato's task —— View job

Claude reviewing PR: chore: fix serverless config

Todo List:

Read repository guidelines (CLAUDE.md)
Analyze changes in examples/counter-serverless/scripts/connect.ts
Analyze changes in packages/next-js/src/mod.ts
Analyze changes in packages/rivetkit/src/registry/mod.ts
Provide comprehensive review feedback
Complete review

Review Feedback

I've analyzed the three files changed in this PR. The changes appear to be serverless configuration adjustments and example script modifications. Here's my comprehensive review:

✅ Code Quality & Best Practices

Positive aspects:

Changes are focused and consistent with serverless environment requirements
Clean, minimal modifications without introducing complexity
Proper use of fallback values and environment variables

⚠️ Potential Issues & Concerns

examples/counter-serverless/scripts/connect.ts:7-19

Commented out functionality: The .connect() call and event listener are commented out:
```
// Before: const counter = client.counter.getOrCreate().connect();
// After:  const counter = client.counter.getOrCreate();
// Before: counter.on("newCount", (count: number) => console.log("Event:", count));
// After:  // counter.on("newCount", (count: number) => console.log("Event:", count));
```
- Impact: This disables real-time event functionality in the example
- Concern: Example may no longer demonstrate full capabilities of the counter actor
- Recommendation: Either remove these lines entirely or add explanatory comments about why they're disabled in serverless context
Missing cleanup:
```
// Before: await counter.dispose();
// After:  // await counter.dispose();
```
- Issue: Could lead to resource leaks if the example is used as a template
- Recommendation: Consider if disposal is needed for serverless environments

packages/next-js/src/mod.ts:14

Port default change:
```
// Before: `http://127.0.0.1:${process.env.PORT ?? 8080}`;
// After:  `http://127.0.0.1:${process.env.PORT ?? 3000}`;
```
- ✅ Positive: Aligns with Next.js standard development port (3000)
- Note: This is a sensible change for developer experience

packages/rivetkit/src/registry/mod.ts:241-246

Critical configuration changes:

Request lifespan unit change:
```
// Before: request_lifespan: customConfig.requestLifespan ?? 15 * 60_000,
// After:  request_lifespan: customConfig.requestLifespan ?? 15 * 60,
```
- ⚠️ Breaking change: This changes from 15 minutes (900,000ms) to 15 minutes (900s)
- Question: What unit does the API expect? This could be either a fix or a regression
- Recommendation: Verify the API specification for request_lifespan units
Slot reduction:
```
// Before: customConfig.slotsPerRunner ?? config.totalSlots ?? 10000,
// After:  customConfig.slotsPerRunner ?? config.totalSlots ?? 1000,
```
- Impact: 90% reduction in default slots per runner (10,000 → 1,000)
- Concern: Could impact throughput capacity under high load
- Trade-off: May improve memory usage and startup time

🔧 Performance Considerations

Positive impacts:

Reduced slots per runner may improve memory usage and cold start times
Better alignment with Next.js defaults improves developer experience

Potential concerns:

Significantly reduced slot capacity could create bottlenecks under load
Example script changes may not reflect real-world usage patterns

🔒 Security

No security concerns identified in these configuration changes.

🧪 Test Coverage

Missing test updates: No test modifications accompany these configuration changes. Consider:

Testing serverless configuration with new default values
Verifying example functionality still works as expected
Unit tests for the configuration parameters

📝 Recommendations

Clarify units: Verify request_lifespan expects seconds vs milliseconds
Document rationale: Add comments explaining why slots were reduced from 10k to 1k
Example clarity: Either remove commented code or add explanatory comments
Add TODO tracking: The changes suggest these are temporary fixes - consider tracking remaining work
Consider environment variables: For critical values like slots_per_runner, consider making them configurable via environment variables

Summary

This PR makes reasonable serverless configuration adjustments, but some changes (particularly the slot reduction and time units) should be validated against intended behavior. The example script changes may reduce its educational value by disabling event handling features.

Overall assessment: ✅ Approve with recommendations - Changes are sensible but would benefit from documentation and verification of the time unit change.

claude · 2025-10-07T20:32:12Z

Claude finished @MasterPtato's task —— View job

Claude reviewing PR: chore: fix serverless config

Todo List:

Read repository guidelines (CLAUDE.md)
Analyze changes in examples/counter-serverless/scripts/connect.ts
Analyze changes in packages/next-js/src/mod.ts
Analyze changes in packages/rivetkit/src/registry/mod.ts
Provide comprehensive review feedback
Complete review

Review Feedback

I've analyzed the three files changed in this PR. The changes appear to be serverless configuration adjustments and example script modifications. Here's my comprehensive review:

✅ Code Quality & Best Practices

Positive aspects:

Changes are focused and consistent with serverless environment requirements
Clean, minimal modifications without introducing complexity
Proper use of fallback values and environment variables
Follows repository coding standards from CLAUDE.md

⚠️ Potential Issues & Concerns

examples/counter-serverless/scripts/connect.ts:9,19

Commented out functionality: Event handling and disposal are commented out:
```
// Line 9: counter.on("newCount", (count: number) => console.log("Event:", count));
// Line 19: await counter.dispose();
```
- Impact: This disables real-time event functionality in the example
- Concern: Example may no longer demonstrate full capabilities of the counter actor
- Recommendation: Either remove these lines entirely or add explanatory comments about why they're disabled in serverless context
Missing cleanup: Commented out disposal could lead to resource leaks if the example is used as a template

packages/next-js/src/mod.ts:14

Port default change:
```
// Before: `http://127.0.0.1:${process.env.PORT ?? 8080}`;
// After:  `http://127.0.0.1:${process.env.PORT ?? 3000}`;
```
- ✅ Positive: Aligns with Next.js standard development port (3000)
- Note: This is a sensible change for developer experience

packages/rivetkit/src/registry/mod.ts:242,245

Critical configuration changes:

Request lifespan change:
```
// Line 242: request_lifespan: customConfig.requestLifespan ?? 15 * 60,
```
- ⚠️ Potential issue: If this changed from milliseconds to seconds, verify the API expects seconds
- Recommendation: Confirm the time unit expectation for the serverless API
Slot reduction:
```
// Line 245: customConfig.slotsPerRunner ?? config.totalSlots ?? 1000,
```
- Impact: If this was reduced from 10,000, it's a 90% reduction in default slots per runner
- Concern: Could impact throughput capacity under high load
- Trade-off: May improve memory usage and startup time for serverless environments

🚀 Performance Considerations

Positive impacts:

Reduced slots per runner may improve memory usage and cold start times
Better alignment with Next.js defaults improves developer experience

Potential concerns:

If slots were significantly reduced, could create bottlenecks under high load
Example script changes may not reflect real-world usage patterns

🔒 Security

No security concerns identified in these configuration changes.

🧪 Test Coverage

Missing test updates: No test modifications accompany these configuration changes. Consider:

Testing serverless configuration with new default values
Verifying example functionality still works as expected
Unit tests for the configuration parameters

📝 Recommendations

Clarify time units: Verify request_lifespan API expects seconds vs milliseconds
Document rationale: Add comments explaining configuration value choices
Example clarity: Either remove commented code or add explanatory comments about serverless-specific behavior
Consider environment variables: For critical values like slots_per_runner, consider making them configurable
Add integration test: Verify the counter-serverless example still works end-to-end

Summary

This PR makes reasonable serverless configuration adjustments. The Next.js port change is clearly beneficial. The main concerns are around ensuring the example still demonstrates intended functionality and confirming the serverless configuration values are optimal for the expected use cases.

Overall assessment: ✅ Approve with minor recommendations - Changes are sensible but would benefit from clarification on time units and example functionality.

claude · 2025-10-07T20:32:19Z

Claude finished @MasterPtato's task —— View job

Claude reviewing PR: chore: fix serverless config

Todo List:

Read repository guidelines (CLAUDE.md)
Analyze changes in examples/counter-serverless/scripts/connect.ts
Analyze changes in packages/next-js/src/mod.ts
Analyze changes in packages/rivetkit/src/registry/mod.ts
Provide comprehensive review feedback
Complete review

Review Feedback

I've analyzed the three files changed in this PR. The changes appear to be serverless configuration adjustments and example script modifications. Here's my comprehensive review:

✅ Code Quality & Best Practices

Positive aspects:

Changes are focused and consistent with serverless environment requirements
Clean, minimal modifications without introducing complexity
Proper use of fallback values and environment variables

⚠️ Potential Issues & Concerns

examples/counter-serverless/scripts/connect.ts:7-19

Commented out functionality: The .connect() call and event listener are commented out:
```
// Before: const counter = client.counter.getOrCreate().connect();
// After:  const counter = client.counter.getOrCreate();
// Before: counter.on("newCount", (count: number) => console.log("Event:", count));
// After:  // counter.on("newCount", (count: number) => console.log("Event:", count));
```
- Impact: This disables real-time event functionality in the example
- Concern: Example may no longer demonstrate full capabilities of the counter actor
- Recommendation: Either remove these lines entirely or add explanatory comments about why they're disabled in serverless context
Missing cleanup:
```
// Before: await counter.dispose();
// After:  // await counter.dispose();
```
- Issue: Could lead to resource leaks if the example is used as a template
- Recommendation: Consider if disposal is needed for serverless environments

packages/next-js/src/mod.ts:14

Port default change:
```
// Before: `http://127.0.0.1:${process.env.PORT ?? 8080}`;
// After:  `http://127.0.0.1:${process.env.PORT ?? 3000}`;
```
- ✅ Positive: Aligns with Next.js standard development port (3000)
- Note: This is a sensible change for developer experience

packages/rivetkit/src/registry/mod.ts:241-246

Critical configuration changes:

Request lifespan unit change:
```
// Before: request_lifespan: customConfig.requestLifespan ?? 15 * 60_000,
// After:  request_lifespan: customConfig.requestLifespan ?? 15 * 60,
```
- ⚠️ Breaking change: This changes from 15 minutes (900,000ms) to 15 minutes (900s)
- Question: What unit does the API expect? This could be either a fix or a regression
- Recommendation: Verify the API specification for request_lifespan units
Slot reduction:
```
// Before: customConfig.slotsPerRunner ?? config.totalSlots ?? 10000,
// After:  customConfig.slotsPerRunner ?? config.totalSlots ?? 1000,
```
- Impact: 90% reduction in default slots per runner (10,000 → 1,000)
- Concern: Could impact throughput capacity under high load
- Trade-off: May improve memory usage and startup time

🔧 Performance Considerations

Positive impacts:

Reduced slots per runner may improve memory usage and cold start times
Better alignment with Next.js defaults improves developer experience

Potential concerns:

Significantly reduced slot capacity could create bottlenecks under load
Example script changes may not reflect real-world usage patterns

🔒 Security

No security concerns identified in these configuration changes.

🧪 Test Coverage

Missing test updates: No test modifications accompany these configuration changes. Consider:

Testing serverless configuration with new default values
Verifying example functionality still works as expected
Unit tests for the configuration parameters

📝 Recommendations

Clarify units: Verify request_lifespan expects seconds vs milliseconds
Document rationale: Add comments explaining why slots were reduced from 10k to 1k
Example clarity: Either remove commented code or add explanatory comments
Add TODO tracking: The changes suggest these are temporary fixes - consider tracking remaining work
Consider environment variables: For critical values like slots_per_runner, consider making them configurable via environment variables

Summary

This PR makes reasonable serverless configuration adjustments, but some changes (particularly the slot reduction and time units) should be validated against intended behavior. The example script changes may reduce its educational value by disabling event handling features.

Overall assessment: ✅ Approve with recommendations - Changes are sensible but would benefit from documentation and verification of the time unit change.

graphite-app · 2025-10-07T23:36:45Z

Merge activity

Oct 7, 11:36 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Oct 7, 11:37 PM UTC: CI is running for this pull request on a draft pull request (#1365) due to your merge queue CI optimization settings.
Oct 7, 11:37 PM UTC: Merged by the Graphite merge queue via draft PR: #1365.

MasterPtato requested a review from NathanFlurry October 6, 2025 23:53

NathanFlurry mentioned this pull request Oct 6, 2025

chore(core): add x-rivet-endpoint header for serverless runners & cleanup severless runner setup #1359

Closed

MasterPtato mentioned this pull request Oct 6, 2025

fix(next): fix local dev #1352

Closed

vercel bot deployed to Preview October 6, 2025 23:56 View deployment

MasterPtato force-pushed the 10-06-chore_fix_serverless_config branch from ae48822 to 3796965 Compare October 7, 2025 17:52

vercel bot deployed to Preview October 7, 2025 18:51 View deployment

chore: fix serverless config

4d2265c

NathanFlurry force-pushed the 10-06-chore_fix_serverless_config branch from 3796965 to 4d2265c Compare October 7, 2025 20:31

NathanFlurry force-pushed the 10-06-chore_core_add_x-rivet-endpoint_header_for_serverless_runners_cleanup_severless_runner_setup branch from 9de93c7 to a022c4f Compare October 7, 2025 20:31

vercel bot deployed to Preview October 7, 2025 21:20 View deployment

This was referenced Oct 7, 2025

chore(core): add smoke test #1363

Closed

chore(core): handle errors inside message handlers #1364

Closed

graphite-app bot pushed a commit that referenced this pull request Oct 7, 2025

chore: fix serverless config (#1361)

17f4a26

graphite-app bot closed this Oct 7, 2025

graphite-app bot deleted the 10-06-chore_fix_serverless_config branch October 7, 2025 23:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore: fix serverless config #1361

chore: fix serverless config #1361

MasterPtato commented Oct 6, 2025

Uh oh!

vercel bot commented Oct 6, 2025 •

edited

Loading

Uh oh!

claude bot commented Oct 6, 2025 •

edited

Loading

Uh oh!

MasterPtato commented Oct 6, 2025 •

edited by NathanFlurry

Loading

Uh oh!

pkg-pr-new bot commented Oct 6, 2025 •

edited

Loading

Uh oh!

claude bot commented Oct 7, 2025 •

edited

Loading

Uh oh!

claude bot commented Oct 7, 2025 •

edited

Loading

Uh oh!

claude bot commented Oct 7, 2025 •

edited

Loading

Uh oh!

graphite-app bot commented Oct 7, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

chore: fix serverless config #1361

chore: fix serverless config #1361

Conversation

MasterPtato commented Oct 6, 2025

Uh oh!

vercel bot commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Oct 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: chore: fix serverless config

Todo List:

Review Feedback

✅ Code Quality & Best Practices

⚠️ Potential Issues

🔧 Configuration Concerns

🚀 Performance Considerations

🔒 Security

🧪 Test Coverage

📝 Recommendations

Summary

Uh oh!

MasterPtato commented Oct 6, 2025 • edited by NathanFlurry Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

How to use the Graphite Merge Queue

Uh oh!

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

Uh oh!

Uh oh!

claude bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo List:

Review Feedback

✅ Code Quality & Best Practices

⚠️ Potential Issues & Concerns

examples/counter-serverless/scripts/connect.ts:7-19

packages/next-js/src/mod.ts:14

packages/rivetkit/src/registry/mod.ts:241-246

🔧 Performance Considerations

🔒 Security

🧪 Test Coverage

📝 Recommendations

Summary

Uh oh!

claude bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo List:

Review Feedback

✅ Code Quality & Best Practices

⚠️ Potential Issues & Concerns

examples/counter-serverless/scripts/connect.ts:9,19

packages/next-js/src/mod.ts:14

packages/rivetkit/src/registry/mod.ts:242,245

🚀 Performance Considerations

🔒 Security

🧪 Test Coverage

📝 Recommendations

Summary

Uh oh!

claude bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Todo List:

Review Feedback

✅ Code Quality & Best Practices

⚠️ Potential Issues & Concerns

examples/counter-serverless/scripts/connect.ts:7-19

packages/next-js/src/mod.ts:14

packages/rivetkit/src/registry/mod.ts:241-246

🔧 Performance Considerations

🔒 Security

🧪 Test Coverage

📝 Recommendations

Summary

Uh oh!

graphite-app bot commented Oct 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merge activity

Uh oh!

Reviewers

Assignees

vercel bot commented Oct 6, 2025 •

edited

Loading

claude bot commented Oct 6, 2025 •

edited

Loading

MasterPtato commented Oct 6, 2025 •

edited by NathanFlurry

Loading

pkg-pr-new bot commented Oct 6, 2025 •

edited

Loading

claude bot commented Oct 7, 2025 •

edited

Loading

claude bot commented Oct 7, 2025 •

edited

Loading

claude bot commented Oct 7, 2025 •

edited

Loading

graphite-app bot commented Oct 7, 2025 •

edited

Loading