feat(engine): actors lifecycle testing #3623

abcxff · 2025-12-11T01:09:43Z

No description provided.

vercel · 2025-12-11T01:09:48Z

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

Project	Deployment	Review	Updated (UTC)
rivetkit-serverless	Error		Dec 25, 2025 2:46am

3 Skipped Deployments

Project	Deployment	Review	Updated (UTC)
rivet-cloud	Ignored		Dec 25, 2025 2:46am
rivet-inspector	Ignored	Preview	Dec 25, 2025 2:46am
rivet-site	Ignored	Preview	Dec 25, 2025 2:46am

abcxff · 2025-12-11T01:10:00Z

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(engine): test actors runner kv #3625
chore(engine): test actor alarms #3624
feat(engine): actors lifecycle testing #3623 👈 (View in Graphite)
chore: setup rivet-engine api tests #3540
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 · 2025-12-11T01:11:59Z

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 2f65035

claude · 2025-12-11T01:14:11Z

PR Review: feat(engine): actors lifecycle testing

Overview

This PR introduces a comprehensive Rust-based test runner infrastructure for Rivet Actors lifecycle testing, replacing the previous TypeScript-based runner. The implementation adds ~2817 lines implementing a WebSocket-based test runner with 22 lifecycle tests.

✅ Strengths

Architecture & Design

Well-structured test infrastructure: The new test_runner module is cleanly separated into runner.rs (core), behaviors.rs (test actors), actor.rs (traits), and protocol.rs (helpers)
Comprehensive test coverage: Tests cover creation, initialization, start timeout, crash policies (Restart/Sleep/Destroy), graceful shutdown, sleep/wake cycles, and exponential backoff
Behavioral composition pattern: Builder pattern with with_actor_behavior allows flexible test setup

Code Quality

Good use of async primitives with tokio::sync::oneshot channels
Consistent structured logging with tracing
Strong typing with ActorStartResult, ActorStopResult enums

⚠️ Issues & Concerns

Critical Issues

1. Debug Code Left in Production (CRITICAL)

Location: engine/packages/runtime/src/term_signal.rs:119-123

Debug log with "hello?" message and commented-out code in production runtime. This needs to be removed or cleaned up.

2. Stale TODO Comments

Location: engine/packages/engine/tests/common/test_runner/runner.rs:818-825

Commented-out test checklist at end of file, but these tests are already implemented. Remove these comments.

3. Synchronous Sleep Blocks Message Loop

Location: engine/packages/engine/tests/common/test_runner/runner.rs:592-607

tokio::time::sleep blocks the entire message loop during ActorStartResult::Delay. If an actor delays 5 seconds, no other messages can be processed. Implement proper async handling or document the limitation.

High Priority Issues

4. Silent Error Handling

Location: Multiple locations in actor.rs

Errors when sending events are silently ignored with let _ = self.event_tx.send(). Should at minimum log errors.

5. Race Condition in Shutdown

Location: engine/packages/engine/tests/common/test_runner/runner.rs:271-278

Race between checking shutdown flag and closing websocket. Consider using CancellationToken.

6. Unbounded Memory Growth

Location: engine/packages/engine/tests/common/test_runner/runner.rs:51

event_history grows unbounded until acks arrive. Add size limit or document assumptions.

Medium Priority Issues

7. Empty Test File

Location: engine/packages/engine/tests/actors_alarms.rs

Empty test file checked in. Either implement or remove.

8. Missing HTTP Tunneling

Location: engine/packages/engine/tests/actors_lifecycle.rs:259-261

Good TODO about missing HTTP tunneling, but unclear if/when it will be implemented. Consider filing tracking issue.

Low Priority

9. Polling Pattern Repetition

Multiple tests use similar polling loops. Consider extracting helpers like poll_until_destroyed, poll_until_connectable.

🔍 Security

No obvious vulnerabilities (test infrastructure)
Proper WebSocket protocol usage
No credential leakage

📊 Performance

actor_start_timeout takes 35+ seconds (properly documented)
50ms polling is reasonable
Unbounded event history could be issue in long tests

🧪 Test Coverage

Excellent coverage: creation, timeouts, all crash policies, shutdown, sleep/wake, backoff

Missing: alarm functionality, HTTP/WebSocket connectivity tests

Recommendations Summary

Must Fix Before Merge

❌ Remove debug code from term_signal.rs:119-123
❌ Remove stale TODO comments at end of runner.rs
❌ Remove or implement actors_alarms.rs empty file

Should Fix Before Merge

⚠️ Address synchronous sleep blocking message loop (or document limitation)
⚠️ Improve error handling for send_event
⚠️ Add bounded memory for event history or document assumptions

Nice to Have

📋 Extract common polling patterns into helpers
📋 File tracking issue for HTTP tunneling

Final Assessment

Overall: Strong implementation with cleanup needed

Well-designed test infrastructure with comprehensive coverage. Main concerns are debug code in production files (must fix) and architectural limitations (should address or track).

Recommendation: Request changes to address critical issues (#1-3), then approve after fixes.

NathanFlurry · 2025-12-25T02:34:06Z

engine/packages/engine/tests/actors_alarms.rs

@@ -0,0 +1 @@
+


graphite-app · 2025-12-25T02:46:16Z

Merge activity

Dec 25, 2:46 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Dec 25, 2:46 AM UTC: CI is running for this pull request on a draft pull request (#3674) due to your merge queue CI optimization settings.
Dec 25, 2:47 AM UTC: Merged by the Graphite merge queue via draft PR: #3674.

abcxff mentioned this pull request Dec 11, 2025

chore: setup rivet-engine api tests #3540

Closed

This was referenced Dec 11, 2025

chore(engine): test actor alarms #3624

Closed

chore(engine): test actors runner kv #3625

Closed

vercel bot had a problem deploying to Preview – rivetkit-serverless December 11, 2025 01:10 Failure

abcxff requested a review from NathanFlurry December 11, 2025 02:18

NathanFlurry force-pushed the 11-10-chore_cleanup_rivet-engine_tests branch from ebaa134 to 681064f Compare December 25, 2025 01:40

NathanFlurry force-pushed the 11-17-feat_engine_actors_lifecycle_testing branch from 2fc28aa to 2f65035 Compare December 25, 2025 01:41

vercel bot had a problem deploying to Preview – rivetkit-serverless December 25, 2025 01:42 Failure

abcxff marked this pull request as ready for review December 25, 2025 01:42

NathanFlurry approved these changes Dec 25, 2025

View reviewed changes

engine/packages/engine/tests/actors_alarms.rs

@@ -0,0 +1 @@

Copy link

Member

NathanFlurry Dec 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

empty file

feat(engine): actors lifecycle testing

270d196

NathanFlurry force-pushed the 11-10-chore_cleanup_rivet-engine_tests branch from 681064f to ccf9736 Compare December 25, 2025 02:45

NathanFlurry force-pushed the 11-17-feat_engine_actors_lifecycle_testing branch from 2f65035 to 270d196 Compare December 25, 2025 02:45

vercel bot had a problem deploying to Preview – rivetkit-serverless December 25, 2025 02:46 Failure

graphite-app bot pushed a commit that referenced this pull request Dec 25, 2025

feat(engine): actors lifecycle testing (#3623)

2855c56

graphite-app bot closed this Dec 25, 2025

graphite-app bot deleted the 11-17-feat_engine_actors_lifecycle_testing branch December 25, 2025 02:47

claude bot mentioned this pull request Dec 25, 2025

[Graphite MQ] Draft PR GROUP:spec_c4d928 (PRs 3540, 3623) #3674

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(engine): actors lifecycle testing #3623

feat(engine): actors lifecycle testing #3623

Uh oh!

abcxff commented Dec 11, 2025

Uh oh!

vercel bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

abcxff commented Dec 11, 2025 •

edited

Loading

Uh oh!

pkg-pr-new bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

claude bot commented Dec 11, 2025 •

edited

Loading

Uh oh!

NathanFlurry Dec 25, 2025

Uh oh!

graphite-app bot commented Dec 25, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat(engine): actors lifecycle testing #3623

feat(engine): actors lifecycle testing #3623

Uh oh!

Conversation

abcxff commented Dec 11, 2025

Uh oh!

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

Uh oh!

Uh oh!

abcxff commented Dec 11, 2025 • edited 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 Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

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

Uh oh!

PR Review: feat(engine): actors lifecycle testing

Overview

✅ Strengths

Architecture & Design

Code Quality

⚠️ Issues & Concerns

Critical Issues

1. Debug Code Left in Production (CRITICAL)

2. Stale TODO Comments

3. Synchronous Sleep Blocks Message Loop

High Priority Issues

4. Silent Error Handling

5. Race Condition in Shutdown

6. Unbounded Memory Growth

Medium Priority Issues

7. Empty Test File

8. Missing HTTP Tunneling

Low Priority

9. Polling Pattern Repetition

🔍 Security

📊 Performance

🧪 Test Coverage

Recommendations Summary

Must Fix Before Merge

Should Fix Before Merge

Nice to Have

Final Assessment

Uh oh!

NathanFlurry Dec 25, 2025

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Merge activity

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel bot commented Dec 11, 2025 •

edited

Loading

abcxff commented Dec 11, 2025 •

edited

Loading

pkg-pr-new bot commented Dec 11, 2025 •

edited

Loading

claude bot commented Dec 11, 2025 •

edited

Loading

graphite-app bot commented Dec 25, 2025 •

edited

Loading