chore(core): add smoke test

[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	Canceled			Oct 7, 2025 10:06pm

[claude]

Claude finished @NathanFlurry's task —— View job

---

Code Review for Smoke Test Addition

I've completed a thorough review of this PR that adds a smoke test example to RivetKit. Here's my comprehensive feedback:

Todo List

• Read repository guidelines (CLAUDE.md)
• Review all changed files for code quality and best practices
• Check for potential bugs and issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage and implementation
• Provide final recommendations

---

✅ Positive Aspects

Excellent Adherence to Project Standards

• Perfect example structure: Follows all CLAUDE.md guidelines for examples
• Correct naming: Uses example-smoke-test pattern as required
• Proper dependencies: Uses workspace:* for rivetkit dependency
• Complete configuration: Includes all required files (turbo.json, tsconfig.json, etc.)

High Code Quality

• TypeScript best practices: Strict mode enabled, proper typing throughout
• Clean architecture: Well-separated concerns with logical file organization
• Modern JavaScript: Uses ESNext features appropriately
• Proper error handling: Comprehensive error collection and reporting

Robust Test Implementation

• Configurable parameters: Environment variable support for test customization
• Performance monitoring: Built-in timing and statistics collection
• Progress tracking: Real-time progress reporting during execution
• Comprehensive metrics: Average, median, min, max duration tracking

---

🐛 Issues Found

1. Import Path Issue (scripts/connect.ts:2)

import type { Registry } from "../src/registry";
//                           ^^^^^^^^^^^^^^
// Should be: "../src/server/registry"

The import path is incorrect - registry.ts is in src/server/ not src/.

2. Missing Dependency (package.json:17)

The pnpm-lock.yaml shows vitest was added but it's not declared in package.json. Either:

• Add vitest to devDependencies if needed
• Remove it from lock file if it's not used

3. Unused README Template (README.md:20)

The README uses generic npm commands instead of the pnpm commands used in the project:

npm install  # Should be: pnpm install
npm run dev  # Should be: pnpm run dev

---

🔧 Minor Improvements

1. Environment Variable Documentation

Consider documenting all environment variables in README.md:

• RUN_DURATION
• SPAWN_ACTOR_INTERVAL
• TOTAL_ACTOR_COUNT (derived)

2. Error Type Safety (spawn-actor.ts:7)

export type SmokeTestError = {
    index: number;
    error: unknown;  // Could be more specific
};

3. Magic Numbers (index.ts:17)

PROGRESS_LOG_INTERVAL_MS = 250 could be configurable for different verbosity levels.

---

⚡ Performance Considerations

• Efficient concurrency: Good use of Promise.all for actor spawning
• Memory management: Proper disposal of actor connections
• Reasonable defaults: 10ms intervals prevent overwhelming the system
• Performance monitoring: Built-in timing helps identify bottlenecks

---

🔒 Security Assessment

• No security concerns: Code doesn't expose sensitive data
• Safe UUID usage: Uses Node.js crypto for test IDs
• Local-only: Hardcoded localhost connection appropriate for testing

---

📊 Test Coverage

Excellent Coverage

• Core functionality: Tests actor creation, increment, disposal
• Error handling: Comprehensive error collection and reporting
• Performance metrics: Duration tracking and statistics
• Configuration: Multiple environment variable options

Potential Additions

• Connection failure scenarios: Test server unavailability
• Concurrent stress testing: Higher actor counts
• Different actor types: Beyond just counter actors

---

🎯 Final Recommendations

Must Fix

1. Fix import path in scripts/connect.ts
2. Resolve vitest dependency issue

Should Fix

3. Update README to use pnpm commands
4. Add environment variable documentation

Consider

5. Make progress logging interval configurable
6. Add more specific error typing

Overall Assessment: This is a high-quality smoke test implementation that follows project conventions excellently. The few issues found are minor and easily fixable. The test provides valuable functionality for validating RivetKit performance and reliability.

[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(core): handle errors inside message handlers #1364
• chore(core): add smoke test #1363 👈 (View in Graphite)
• chore: fix serverless config #1361
• 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.

[claude]

Claude finished @NathanFlurry's task —— View job

---

Code Review for Smoke Test Addition

I've completed a thorough review of this PR that adds a smoke test example to RivetKit. Here's my comprehensive feedback:

Todo List

• Read repository guidelines (CLAUDE.md)
• Review all changed files for code quality and best practices
• Check for potential bugs and issues
• Evaluate performance considerations
• Assess security concerns
• Review test coverage and implementation
• Provide final recommendations

---

✅ Positive Aspects

Excellent Adherence to Project Standards

• Perfect example structure: Follows all CLAUDE.md guidelines for examples
• Correct naming: Uses example-smoke-test pattern as required
• Proper dependencies: Uses workspace:* for rivetkit dependency
• Complete configuration: Includes all required files (turbo.json, tsconfig.json, etc.)

High Code Quality

• TypeScript best practices: Strict mode enabled, proper typing throughout
• Clean architecture: Well-separated concerns with logical file organization
• Modern JavaScript: Uses ESNext features appropriately
• Proper error handling: Comprehensive error collection and reporting

Robust Test Implementation

• Configurable parameters: Environment variable support for test customization
• Performance monitoring: Built-in timing and statistics collection
• Progress tracking: Real-time progress reporting during execution
• Comprehensive metrics: Average, median, min, max duration tracking

---

🐛 Issues Found

1. Import Path Issue (scripts/connect.ts:2)

import type { Registry } from "../src/registry";
//                           ^^^^^^^^^^^^^^
// Should be: "../src/server/registry"

The import path is incorrect - registry.ts is in src/server/ not src/.

2. Missing Dependency (package.json:17)

The pnpm-lock.yaml shows vitest was added but it's not declared in package.json. Either:

• Add vitest to devDependencies if needed
• Remove it from lock file if it's not used

3. Unused README Template (README.md:20)

The README uses generic npm commands instead of the pnpm commands used in the project:

npm install  # Should be: pnpm install
npm run dev  # Should be: pnpm run dev

---

🔧 Minor Improvements

1. Environment Variable Documentation

Consider documenting all environment variables in README.md:

• RUN_DURATION
• SPAWN_ACTOR_INTERVAL
• TOTAL_ACTOR_COUNT (derived)

2. Error Type Safety (spawn-actor.ts:7)

export type SmokeTestError = {
    index: number;
    error: unknown;  // Could be more specific
};

3. Magic Numbers (index.ts:17)

PROGRESS_LOG_INTERVAL_MS = 250 could be configurable for different verbosity levels.

---

⚡ Performance Considerations

• Efficient concurrency: Good use of Promise.all for actor spawning
• Memory management: Proper disposal of actor connections
• Reasonable defaults: 10ms intervals prevent overwhelming the system
• Performance monitoring: Built-in timing helps identify bottlenecks

---

🔒 Security Assessment

• No security concerns: Code doesn't expose sensitive data
• Safe UUID usage: Uses Node.js crypto for test IDs
• Local-only: Hardcoded localhost connection appropriate for testing

---

📊 Test Coverage

Excellent Coverage

• Core functionality: Tests actor creation, increment, disposal
• Error handling: Comprehensive error collection and reporting
• Performance metrics: Duration tracking and statistics
• Configuration: Multiple environment variable options

Potential Additions

• Connection failure scenarios: Test server unavailability
• Concurrent stress testing: Higher actor counts
• Different actor types: Beyond just counter actors

---

🎯 Final Recommendations

Must Fix

1. Fix import path in scripts/connect.ts
2. Resolve vitest dependency issue

Should Fix

3. Update README to use pnpm commands
4. Add environment variable documentation

Consider

5. Make progress logging interval configurable
6. Add more specific error typing

Overall Assessment: This is a high-quality smoke test implementation that follows project conventions excellently. The few issues found are minor and easily fixable. The test provides valuable functionality for validating RivetKit performance and reliability.

---

[graphite-app]

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.

[graphite-app]

There's a discrepancy between the README and implementation regarding configuration options. The README mentions TOTAL_ACTOR_COUNT as a configurable environment variable:

Set `TOTAL_ACTOR_COUNT` and `SPAWN_ACTOR_INTERVAL` environment variables to adjust the workload.

However, in the code, TOTAL_ACTOR_COUNT is calculated from RUN_DURATION and SPAWN_ACTOR_INTERVAL:

const TOTAL_ACTOR_COUNT = Math.ceil(RUN_DURATION / SPAWN_ACTOR_INTERVAL);

To resolve this inconsistency, either:

1. Update the code to accept TOTAL_ACTOR_COUNT directly as an environment variable, or
2. Update the README to mention RUN_DURATION instead of TOTAL_ACTOR_COUNT

This will ensure the documentation accurately reflects the available configuration options.

Suggested change

	const RUN_DURATION = parseEnvInt(process.env.RUN_DURATION, 10_000);
	const SPAWN_ACTOR_INTERVAL = parseEnvInt(process.env.SPAWN_ACTOR_INTERVAL, 10);
	const TOTAL_ACTOR_COUNT = Math.ceil(RUN_DURATION / SPAWN_ACTOR_INTERVAL);
	const RUN_DURATION = parseEnvInt(process.env.RUN_DURATION, 10_000);
	const SPAWN_ACTOR_INTERVAL = parseEnvInt(process.env.SPAWN_ACTOR_INTERVAL, 10);
	const TOTAL_ACTOR_COUNT = process.env.TOTAL_ACTOR_COUNT
	? parseEnvInt(process.env.TOTAL_ACTOR_COUNT, Math.ceil(RUN_DURATION / SPAWN_ACTOR_INTERVAL))
	: Math.ceil(RUN_DURATION / SPAWN_ACTOR_INTERVAL);

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

The vitest dependency appears in the pnpm-lock.yaml file but is missing from the package.json devDependencies section. To maintain consistency between the lock file and package manifest, please add "vitest": "^3.1.1" to the devDependencies object.

Suggested change

	"devDependencies": {
	"rivetkit": "workspace:*",
	"@types/node": "^22.13.9",
	"tsx": "^3.12.7",
	"typescript": "^5.7.3"
	},
	"devDependencies": {
	"rivetkit": "workspace:*",
	"@types/node": "^22.13.9",
	"tsx": "^3.12.7",
	"typescript": "^5.7.3",
	"vitest": "^3.1.1"
	},

Spotted by Diamond



Is this helpful? React 👍 or 👎 to let us know.