chore: setup rivet-engine api tests

[abcxff]

No description provided.

[vercel]

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]

• chore(engine): test actors runner kv #3625
• chore(engine): test actor alarms #3624
• feat(engine): actors lifecycle testing #3623
• chore: setup rivet-engine api tests #3540 👈 (View in Graphite)
• 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.

[claude]

PR Review: Setup rivet-engine API tests

Summary

This PR reorganizes the rivet-engine test suite by:

• Consolidating duplicated test code into helper functions
• Extracting API type definitions to rivet-api-types for reuse
• Creating dedicated API client modules (common/api/public.rs and common/api/peer.rs)
• Reorganizing tests into clearer categories (api_actors_, api_namespaces_, etc.)
• Reducing ~4k lines while adding ~8.5k lines of more structured test code

Overall, this is a solid refactoring that improves test maintainability. Below are specific findings:

---

Code Quality & Best Practices

Positive Highlights

1. Good code deduplication: Extracting common API types to rivet-api-types package eliminates duplication between public and peer APIs
2. Structured test helpers: The new common/api/ modules provide clean, reusable API client functions
3. Consistent error handling: Uses parse_response helper with proper context
4. Type safety: Leverages strong typing throughout with proper serde derives

Issues & Recommendations

1. Missing Derives on Path Structs

Location: engine/packages/api-types/src/actors/delete.rs:12-16

DeletePath has Deserialize but this struct is only used in path parameters (extracted from URL), not deserialized from JSON. Consider adding Clone and Debug for consistency with query params.

2. Inconsistent Reference Usage in URL Building

Location: engine/packages/engine/tests/common/api/ modules

In public.rs:

serde_html_form::to_string(&query)?  // No reference

In peer.rs:

&serde_html_form::to_string(&query)?  // Extra reference

Issue: The peer.rs file uses & before serde_html_form::to_string() while public.rs doesn't. The & is unnecessary.

Recommendation: Remove the unnecessary & in peer.rs for consistency (lines 30, 72, 97, 123, 147, 170, 217, 240, 261).

3. Different Error Context Messages

Location: engine/packages/engine/tests/common/api/

public.rs:18:

Ok(response.json().await.context("failed to parse response")?)

peer.rs:17:

Ok(response.json().await?)  // No context

Recommendation: Add .context("failed to parse response") to peer.rs:17 for consistency.

---

Potential Bugs

1. Cursor-based Pagination Logic Issue ⚠️

Location: engine/packages/api-public/src/actors/list.rs:113-120

// Apply cursor filtering if provided
if let Some(cursor_str) = &query.cursor {
    let cursor_ts: i64 = cursor_str.parse().context("invalid cursor format")?;
    actors.retain(|actor| actor.create_ts < cursor_ts);  // ⚠️ Issue here
}

// Apply limit after cursor filtering
let limit = query.limit.unwrap_or(100);
actors.truncate(limit);

Issue: The cursor filtering happens AFTER fetching actors by IDs. This means:

1. If you have 32 actor IDs but cursor filters most of them out, you might get very few results
2. The cursor is being applied client-side to an already-filtered set, not server-side
3. This differs from the fanout path where cursor is passed through to peer API

Impact: Pagination may not work correctly when using actor_id parameter with cursors.

Recommendation: Either:

• Document that cursors don't work well with direct actor_id queries, OR
• Pass cursor to fetch_actors_by_ids to filter server-side, OR
• Remove cursor support for the actor_id path entirely

2. Potential Race Condition in Test

Location: engine/packages/engine/tests/api_actors_list.rs:72-199

The list_with_pagination test creates 5 actors sequentially, then assumes they'll be ordered by create_ts descending.

Issue: If actors are created very quickly (within the same millisecond), they may have identical create_ts values, making the ordering non-deterministic.

Impact: Test could be flaky, especially in fast CI environments.

Recommendation: Add small delays between creates OR document that the test uses >= for ordering (which it already does at line 184).

---

Performance Considerations

1. Fanout Behavior Change

Location: engine/packages/api-public/src/runners.rs:89-97

The code now passes the full query to fanout instead of constructing a peer-specific query. Both public and peer APIs now use the same type definitions from rivet-api-types.

Recommendation: ✅ This looks correct and improves consistency.

2. Truncation After Fanout

Location: engine/packages/api-public/src/actors/list.rs:204

When fanning out to multiple datacenters, you fetch up to limit actors from each datacenter, merge them, sort, then truncate. This means you could fetch N_datacenters * limit actors only to discard most.

Impact: Moderate - increases network traffic and memory usage during fanout.

Recommendation: This is probably acceptable for now, but consider documenting this behavior.

---

Security Concerns

1. Actor ID Limit Enforcement ✅

Location: engine/packages/api-public/src/actors/list.rs:86-93

// Cap actor_ids count to 32
if actor_ids.len() > 32 {
    return Err(errors::Validation::TooManyActorIds {
        max: 32,
        count: actor_ids.len(),
    }
    .build());
}

✅ Good: Proper input validation to prevent abuse.

2. Namespace Fields Now Public

Location: engine/packages/api-peer/src/namespaces.rs:72-73

Fields changed from private to pub in CreateRequest struct.

Impact: Low - likely needed for test helper functions.

Recommendation: ✅ Acceptable.

---

Test Coverage

Positive Changes

1. More comprehensive test scenarios:

   • Pagination tests with cursor handling
   • Edge cases like timestamp validation
   • Multi-datacenter fanout scenarios

2. Better test organization:

   • Separated by API endpoint (api_actors_, api_namespaces_)
   • Lifecycle tests consolidated

Gaps

1. No negative test cases: Most tests focus on happy paths
2. Error path coverage: Limited testing of error responses

Recommendation: Add negative test cases in follow-up PRs.

---

Architecture & Design

Type Consolidation Strategy ✅

The PR successfully consolidates types into rivet-api-types:

• ✅ actors::delete::{DeleteQuery, DeletePath, DeleteResponse}
• ✅ runners::list_names::{ListNamesQuery, ListNamesResponse}
• ✅ Other list types

Benefit: Eliminates duplication, ensures public and peer APIs stay in sync.

API Client Pattern ✅

The new common/api/{public,peer}.rs modules follow a consistent pattern:

pub async fn build_{resource}_{action}_request(...) -> Result<RequestBuilder>
pub async fn {resource}_{action}(...) -> Result<Response>

Benefit: Clean separation of request building and execution, easy to test, reusable.

Recommendation: ✅ Good pattern. Consider documenting this in test README.

---

Minor Issues

1. Unused imports: #![allow(dead_code, unused_variables)] at top of api modules - verify if all functions are used
2. Version bump: Cargo.lock shows version change from 2.0.29-rc.1 to 2.0.29 - confirm this is intentional

---

Recommendations Summary

Must Fix (Before Merge)

1. ❗ Fix or document cursor pagination logic for actor_id queries (Bug [SVC-2555] Set up issue templates #1)
2. ❗ Make error handling consistent between public.rs and peer.rs

Should Fix (Before or Soon After Merge)

3. Remove unnecessary & references in peer.rs URL building
4. Verify all dead_code allowances are necessary

Nice to Have (Future PRs)

5. Add negative test cases for error paths
6. Document the test helper patterns
7. Consider optimizing fanout truncation behavior

---

Conclusion

This is a solid refactoring that significantly improves test organization and maintainability. The type consolidation and API client helpers are well-designed. The main concern is the cursor pagination logic for direct actor ID queries which needs clarification or fixes.

After addressing the cursor pagination issue and consistency points, this PR should be good to merge.

Approval Status: ⚠️ Needs minor fixes before merge

Great work on the cleanup! 🎉

[pkg-pr-new]

More templates

• actor-actions
• example-ai-agent
• example-ai-and-user-generated-actors-freestyle
• chat-room
• chat-room-next-js
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-cloudflare-workers-inline-client
• cross-actor-actions
• example-cursors
• example-cursors-raw-websocket
• custom-serverless
• example-deno
• example-sqlite
• example-elysia
• example-experimental-durable-streams-ai-agent
• example-express
• example-hono
• hono-bun
• hono-react
• example-kitchen-sink
• multi-region
• native-websockets
• next-js
• example-node
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• scheduling
• state
• example-stream
• example-trpc

@rivetkit/cloudflare-workers

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: 681064f

[graphite-app]

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.