fix(serverless): general cleanup

[MasterPtato]

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 11, 2025 11:54pm

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 11, 2025 11:54pm
rivet-inspector	Ignored	Preview		Nov 11, 2025 11:54pm
rivet-site	Ignored	Preview		Nov 11, 2025 11:54pm

[MasterPtato]

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

• fix(serverless): general cleanup #3450 👈 (View in Graphite)
• feat(pegboard): move pegboard serverless to wf #3416
• 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.

[pkg-pr-new]

More templates

• example-ai-agent
• example-background-jobs
• example-better-auth-external-db
• example-bots
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-cursors
• example-cursors-raw-websocket
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• example-game
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc
• example-workflows

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 5c32999

[claude]

Review complete - see full review below

[claude]

PR Review: fix(serverless): general cleanup

Summary

This PR performs cleanup of the serverless runner implementation, removing a massive number of corrupt/malformed error artifact files (90 files) and refactoring the serverless workflow code for better clarity and maintainability.

Code Quality & Best Practices

Positive Changes:

• Excellent cleanup of error artifacts: Removed 90+ corrupted error JSON files with malformed names
• Improved code organization: The serverless workflow refactoring separates concerns better between pool.rs, runner.rs, and connection.rs
• Better state management: Introduction of explicit State struct in runner.rs with is_draining flag improves lifecycle tracking
• Proper workflow separation: Connection lifecycle is now managed in a separate workflow

Concerns:

1. Error handling in connection.rs:266 - Silently ignoring errors could benefit from at least logging
2. Logging style in pool.rs:162-167 - Uses ? debug formatting instead of % for IDs per CLAUDE.md guidance

[claude]

Potential Bugs

1. Race condition potential in connection.rs:221-237 - The select! block could have a race where the stream ends unexpectedly before we send the drain signal. The TODO comment on lines 225-229 acknowledges this known issue but it should be tracked and addressed.

2. Integer overflow protection in pool.rs:173-178 - The code correctly uses saturating_sub but there is no validation that runners_margin, min_runners, or max_runners are actually within safe ranges before the calculation.

Performance Considerations

1. Activity retry configuration in connection.rs:72-73 - Using usize::MAX retries seems excessive and could lead to infinite retry loops. Consider a more reasonable upper bound.

2. Parallel operation in connection.rs:85-93 - Good use of tokio::try_join! to fetch runner config and namespace data in parallel.

3. Stream processing in connection.rs:176-217 - Well-structured, but could benefit from a max message size check when decoding base64 to prevent memory exhaustion attacks.

[claude]

Security Concerns

1. Header injection in connection.rs:131-164 - The code properly validates headers by attempting to parse them as HeaderName and HeaderValue, silently filtering invalid ones. This is good for security, but the silent filtering could hide configuration errors. Consider logging invalid headers at debug level.

2. URL validation in upsert.rs:103-107 - Good validation of the serverless URL using url::Url::parse.

3. Header limits in upsert.rs:109-136 - Excellent security validation:

   • Max 16 headers
   • Header names max 128 chars
   • Header values max 4096 chars
   • Proper parsing validation

Test Coverage

Missing test coverage: This PR does not include any test changes or additions. Given the refactoring scope, integration tests should verify:

• Pool scaling up/down behavior
• Runner drain lifecycle
• Connection timeout and retry behavior
• Error handling for malformed SSE messages
• State transitions during draining

[claude]

Recommendations

1. Add logging for the ignored error in connection.rs:266
2. Fix the logging format in pool.rs:162-167 to use % for IDs per CLAUDE.md
3. Consider more reasonable retry limits for the outbound request activity
4. Add test coverage for the refactored serverless workflows
5. Create a GitHub issue to track the lifecycle correlation problem mentioned in the TODO
6. Consider logging filtered-out invalid headers at debug level in connection.rs:134

Verdict

✅ Approve with minor suggestions

This is a solid cleanup PR that removes technical debt and improves code organization. The core logic changes are well-thought-out and the separation of concerns is clearer. The minor issues noted above are primarily about logging consistency and future improvements rather than blocking problems.

The removal of 90+ corrupt error files alone makes this a valuable cleanup, and the refactoring sets a better foundation for future serverless runner work.

[claude]

This is a solid refactoring that improves code clarity and maintainability of the serverless runner infrastructure. Key improvements include: (1) Simplified architecture - connection workflow now fetches its own config instead of receiving it as parameters, reducing coupling. (2) Improved signal flow - eliminated ConnectionDrainStarted forwarding through runner workflow; connection now signals pool directly. (3) Better state management - runner workflow tracks draining state properly. (4) Code quality - removed unused imports, fixed pegboard_runner_config__get typo (double underscore), cleaned up 90+ corrupt error artifacts. (5) Simplified control flow - removed repeat loop and join_signal macro from runner workflow.

Main concern: Potential race condition at connection.rs:75 - if runner workflow has not completed InitState activity yet, parse_state will fail at connection.rs:282. Consider adding error handling for this case.

Other observations: (1) connection.rs:72-73 uses infinite timeout and retries which may mask errors. (2) Potential duplicate RunnerDrainStarted signal sending between inline send and fallback. (3) Header filtering at connection.rs:131-139 is good for security but silent - consider logging filtered headers. (4) Sequential runner startup at pool.rs:69-82 could be slow for large start_count - consider parallel dispatch. (5) Verify error generation tooling was fixed to prevent corrupt artifacts.

Code follows CLAUDE.md conventions well: structured logging, anyhow::Context for errors, lowercase log messages, hard tabs, proper Result types. Tag ordering standardized to namespace_id then runner_name.

Recommendation: Approve with minor fixes for initialization race condition. Great work on eliminating complex signal forwarding!