Skip to content

feat(sandbox): add test counter and SQLite load actors#4500

Merged
NathanFlurry merged 11 commits intomainfrom
feat/sandbox-test-actors
Mar 25, 2026
Merged

feat(sandbox): add test counter and SQLite load actors#4500
NathanFlurry merged 11 commits intomainfrom
feat/sandbox-test-actors

Conversation

@NathanFlurry
Copy link
Member

Summary

  • Add testCounter actor: simple in-memory state counter with increment/getCount/reset
  • Add testCounterSqlite actor: same counter interface backed by SQLite via rivetkit/db
  • Add testSqliteLoad actor: 50 migrations in onMigrate, runLoadTest action runs 20 diverse SQLite queries (inserts, selects, joins, aggregates, updates)

Test plan

  • Verified all three actors work e2e locally via curl against the gateway API
  • Confirm actors appear in sandbox inspector UI

🤖 Generated with Claude Code

@railway-app
Copy link

railway-app bot commented Mar 24, 2026

🚅 Deployed to the rivet-pr-4500 environment in rivet-frontend

Service Status Web Updated (UTC)
kitchen-sink ❌ Build Failed (View Logs) Web Mar 25, 2026 at 8:12 pm
website 🕒 Building (View Logs) Web Mar 25, 2026 at 8:12 pm
frontend-inspector ✅ Success (View Logs) Web Mar 25, 2026 at 8:11 pm
frontend-cloud ✅ Success (View Logs) Web Mar 25, 2026 at 8:11 pm
ladle ❌ Build Failed (View Logs) Web Mar 25, 2026 at 8:10 pm
mcp-hub ✅ Success (View Logs) Web Mar 24, 2026 at 8:55 pm

@claude
Copy link

claude bot commented Mar 24, 2026

PR Review: feat(sandbox): add test counter and SQLite load actors

Summary

This PR adds benchmark tooling and three test/demo actors to the sandbox example. Overall the changes are clean and well-structured. One critical security issue needs to be addressed before merging.


🚨 Critical: API Key Committed to Repository

Files: examples/sandbox/scripts/after-no-margin.txt, examples/sandbox/scripts/after-with-margin.txt

Both benchmark output files contain a live API key embedded in the command output:

tsx scripts/benchmark.ts --endpoint https://kitchen-sink-j1x7-production-yl5l:pk_iZlRErFUNwBNcXKuo1i5dfXP9OWLUcE2uh5tvkMvGf88PAI0xOWXeHa46PKAzvej@api.staging.rivet.dev

The token pk_iZlRErFUNwBNcXKuo1i5dfXP9OWLUcE2uh5tvkMvGf88PAI0xOWXeHa46PKAzvej looks like a real Rivet staging API key. This should be revoked immediately and scrubbed from these files before merging. Replace with a placeholder like pk_YOUR_API_KEY_HERE in the example output.


Issues

Package version pinning deviates from monorepo conventions

examples/sandbox/package.json pins rivetkit and @rivetkit/react to 2.1.7, but CLAUDE.md says:

When adding RivetKit dependencies to examples in /examples/, use * as the version. The root resolutions will automatically resolve these to the local workspace packages.

If this is intentionally testing against a specific published release rather than local workspace packages, a comment explaining the deviation would help.

Nested pnpm-lock.yaml in example subdirectory

A pnpm-lock.yaml was added at examples/sandbox/pnpm-lock.yaml. For a workspace monorepo this is unusual and could diverge from the root lock file. Unless the sandbox is intentionally deployed standalone with published packages, this should be managed via the root workspace lock file.

testSqliteLoad: schema_version table is never used

// Migration 1: schema version tracking
await db.execute(`
  CREATE TABLE IF NOT EXISTS schema_version (
    id INTEGER PRIMARY KEY CHECK (id = 1),
    version INTEGER NOT NULL DEFAULT 50
  )
`);

The schema_version table is created but never queried or updated. Since onMigrate runs all 50 migrations unconditionally (protected by IF NOT EXISTS), the version table does nothing. The comment "Migration 1: schema version tracking" implies it controls migration flow, but it doesn't. Either remove it or add a note clarifying it is just part of the load test schema.


Positive Observations

  • Clean architecture: Extracting registry.serve() into server.ts and removing registry.start() from index.ts is a good improvement.
  • Dockerfile: Multi-stage build now copies only the required directories instead of the full repo context, reducing build times.
  • Frontend fix: rivetEndpoint in App.tsx correctly handles dev vs. production routing.
  • Benchmark script: Well-structured with proper arg parsing, DNS-based baseline, warm-up runs, and clear output formatting.
  • Actor implementations: testCounter and testCounterSqlite are concise and correct. The singleton row pattern (CHECK (id = 1)) in the SQLite actor is idiomatic.

@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 25, 2026

More templates

@rivetkit/cloudflare-workers

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/sqlite-vfs

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/sqlite-vfs@4500

@rivetkit/traces

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

@rivetkit/workflow-engine

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/workflow-engine@4500

@rivetkit/virtual-websocket

pnpm add https://pkg.pr.new/rivet-dev/rivet/@rivetkit/virtual-websocket@4500

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 625d388

@claude
Copy link

claude bot commented Mar 25, 2026

PR Review: feat(sandbox): add test counter and SQLite load actors

Critical Issues

1. Leaked Staging API Token in Committed Files

Files: examples/sandbox/scripts/before.txt, examples/sandbox/scripts/after-no-margin.txt, examples/sandbox/scripts/after-with-margin.txt

All three benchmark result files contain a staging API token embedded in the recorded command. The token pk_iZlRErFUNwBNcXKuo1i5dfXP9OWLUcE2uh5tvkMvGf88PAI0xOWXeHa46PKAzvej is a project key embedded in the endpoint URL. Even if it has been rotated, committing credentials to a public repository is a serious security concern. These files also expose a staging namespace name and a server IP address (34.96.106.109). These .txt files should not be committed at all. If baseline results are needed for documentation, scrub sensitive data and add a .gitignore entry to prevent future accidental commits.


Significant Issues

2. Pinned RivetKit Versions in Example

File: examples/sandbox/package.json

Per CLAUDE.md: "When adding RivetKit dependencies to examples in /examples/, use * as the version. The root resolutions will automatically resolve these to the local workspace packages."

"@rivetkit/react": "2.1.7" and "rivetkit": "2.1.7" should both be "*". Pinning to a specific version means this example builds against the published npm version rather than the monorepo source.

3. Standalone pnpm-lock.yaml in Example Directory

File: examples/sandbox/pnpm-lock.yaml (newly added, 4,000+ lines)

This conflicts with the workspace-based architecture. Having two lockfiles risks divergence and maintenance confusion. This appears to be a side effect of the Dockerfile refactor and should be addressed.

4. Dockerfile Loses Multi-Stage Build and pnpm Cache

File: examples/sandbox/Dockerfile

The new single-stage Dockerfile removes the two-stage build (build + production image), meaning dev dependencies like drizzle-kit, typescript, and vite will now be included in the final image, significantly increasing its size. It also removes the --mount=type=cache for the pnpm store, slowing down CI/CD builds. The port change from 3000 to 8080 is also unexplained.

5. .dockerignore at Wrong Path for Build Context

File: examples/sandbox/.dockerignore

The build script uses the repo root as the Docker build context. Docker looks for .dockerignore relative to the build context root, not the Dockerfile location. The newly added .dockerignore inside examples/sandbox/ will have no effect. It should be at the repo root or the build context should be updated.

6. Commented-Out Dead Code in benchmark.ts

The benchmarkWithoutConnection function and NoConnectionBenchmarkResult interface exist but are never used. The code path is entirely commented out. Remove the dead code or reinstate it with a comment explaining why it is disabled.


Minor Issues

7. Schema Version Mismatch in testSqliteLoad

The migration contains 51 steps (migrations 1-51), but the schema_version table is initialized with version = 50. This is inaccurate.

8. testSqliteLoad.runLoadTest() Never Called from Benchmark

The benchmark calls only increment() on all three actors. The runLoadTest() action (which exercises the 50-migration schema and 20-query loop) is never invoked, so the benchmark results do not actually measure SQLite load performance.

9. as any Cast in Benchmark Client Access

(client as any)[actorName] loses type safety. A narrowed union or typed lookup map would be better, even in a script context.


Positive Notes

  • The Rust change in runner_pool.rs is clean: early-exit before spawning sidecar workflows when there is no serverless runner config is well-motivated and correctly versioned with ctx.v(4).
  • testCounter and testCounterSqlite are minimal, appropriate testing fixtures.
  • testCounterSqlite correctly uses the CHECK (id = 1) constraint pattern for a single-row counter table.
  • benchmark.ts is well-structured with proper interfaces, median/average calculations, baseline RTT measurement, and finally-block cleanup.
  • The App.tsx endpoint change (DEV check with window.location.origin fallback) is the correct pattern for deploying behind a reverse proxy.

The leaked token is the blocker. The .txt files with the embedded credential must be removed or scrubbed before merge, and the token should be rotated if it has not been already.

Copy link
Member Author

NathanFlurry commented Mar 25, 2026

Merge activity

  • Mar 25, 7:43 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Mar 25, 8:10 PM UTC: Graphite rebased this pull request as part of a merge.
  • Mar 25, 8:12 PM UTC: Graphite rebased this pull request as part of a merge.
  • Mar 25, 8:12 PM UTC: @NathanFlurry merged this pull request with Graphite.

@NathanFlurry NathanFlurry changed the base branch from 03-24-chore_actor_epoxy_scoped_consensus to graphite-base/4500 March 25, 2026 20:06
@NathanFlurry NathanFlurry changed the base branch from graphite-base/4500 to main March 25, 2026 20:08
@NathanFlurry NathanFlurry force-pushed the feat/sandbox-test-actors branch from 625d388 to 12b3eef Compare March 25, 2026 20:09
@NathanFlurry NathanFlurry force-pushed the feat/sandbox-test-actors branch from 12b3eef to f350b3d Compare March 25, 2026 20:11
@NathanFlurry NathanFlurry merged commit edb2c13 into main Mar 25, 2026
9 of 12 checks passed
@NathanFlurry NathanFlurry deleted the feat/sandbox-test-actors branch March 25, 2026 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant