chore(rivetkit): add listing actors by only name to manager api #3430

NathanFlurry · 2025-11-10T01:12:15Z

No description provided.

vercel · 2025-11-10T01:12:18Z

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Nov 13, 2025 8:55pm

3 Skipped Deployments

Project	Deployment	Preview	Updated (UTC)
rivet-cloud	Ignored		Nov 13, 2025 8:55pm
rivet-inspector	Ignored	Preview	Nov 13, 2025 8:55pm
rivet-site	Ignored	Preview	Nov 13, 2025 8:55pm

NathanFlurry · 2025-11-10T01:12:41Z

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

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-11-10T01:14:50Z

More templates

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: a8f3a9c

claude · 2025-11-13T01:35:49Z

Code Review

Thank you for this PR! I've reviewed the changes adding the listActors functionality to the manager API. Here's my feedback:

✅ Positive Aspects

Consistent API Design: The new listActors method follows the existing patterns in the codebase with proper input types and output structures.
Good Type Safety: TypeScript interfaces are properly defined with ListActorsInput and exported through the driver helpers.
Proper Sorting: The FileSystem driver implementation correctly sorts results by createTs in descending order (most recent first).
Proper Router Logic: The router correctly handles the new path where only name is provided without actor_ids or key.

🔍 Issues & Concerns

1. Missing Test Coverage (High Priority)

The PR adds a new API endpoint but doesn't include any tests. According to CLAUDE.md guidelines, tests should be added. Please add:

Unit tests for the listActors method in the FileSystem driver
Integration tests for the new API endpoint path (GET /actors?name=X)
Tests covering edge cases (empty results, multiple actors with same name, sorting validation)

2. Incomplete Cloudflare Workers Implementation (Medium Priority)

cloudflare-workers/src/manager-driver.ts:352-358

The Cloudflare Workers driver returns an empty array with just a warning. This could silently fail in production:

async listActors({ c, name }: ListActorsInput): Promise<ActorOutput[]> {
    logger().warn({
        msg: "listActors not fully implemented for Cloudflare Workers",
        name,
    });
    return [];
}

Recommendation: Either implement this properly using KV list operations or throw an Unsupported error to make it clear this feature isn't available:

throw new Unsupported("listActors");

3. Unused Parameters in ListActorsInput (Low Priority)

manager/driver.ts:96-100

The ListActorsInput interface defines key and includeDestroyed parameters that are never used:

export interface ListActorsInput<E extends Env = any> {
    c?: HonoContext | undefined;
    name: string;
    key?: string;  // Not used anywhere
    includeDestroyed?: boolean;  // Not used anywhere
}

Recommendation: Either implement filtering by these parameters or remove them to avoid confusion. Keeping unused parameters in the interface suggests future functionality but could mislead other developers.

4. Potential Performance Issue (Medium Priority)

drivers/file-system/manager.ts:337-359

The listActors implementation iterates through ALL actors and filters in-memory:

const itr = this.#state.getActorsIterator({});
for await (const actor of itr) {
    if (actor.name === name) {
        actors.push({...});
    }
}

Concerns:

No pagination support
No limit on results returned
Full table scan with O(n) complexity

Recommendation:

Add pagination parameters (limit, cursor)
Consider adding an index on actor name if this becomes a performance bottleneck
Add a maximum result limit to prevent memory issues

5. Missing Optional Timestamp (Low Priority)

manager/driver.ts:104-108

The createTs field is optional in ActorOutput:

export interface ActorOutput {
    actorId: string;
    name: string;
    key: ActorKey;
    createTs?: number;
}

But in router.ts:732, it falls back to Date.now() which could give incorrect timestamps:

create_ts: actor.createTs ?? Date.now(),

Recommendation: Ensure all drivers consistently populate createTs. The fallback to Date.now() means the API could return the current timestamp instead of the actual creation time, which is misleading.

6. Inconsistent Error Handling (Low Priority)

The validation logic in router.ts:354-361 could be clearer. The nested if-else structure makes it hard to follow the different query parameter combinations.

Recommendation: Add code comments documenting the three supported query patterns:

actor_ids only (with optional name for optimization)
name + key for exact match
name only for listing all actors with that name

📝 Minor Suggestions

Code Comment: Add a comment in router.ts:363-370 explaining this is the "list by name" path
Logging: Consider adding debug logging in the router when the list path is taken
Documentation: Update any API documentation to reflect this new query capability

🔒 Security

No security concerns identified. The implementation properly validates inputs and doesn't introduce any obvious vulnerabilities.

🎯 Summary

This is a solid foundation for the listActors functionality, but it needs:

Must have: Test coverage
Should have: Complete Cloudflare implementation or explicit error
Should have: Pagination support for FileSystem driver
Nice to have: Remove unused parameters or implement their functionality

The code quality is good and follows the project conventions. Once the test coverage is added and the performance/pagination concerns are addressed, this will be ready to merge.

Would you like me to help write tests for this functionality?

claude · 2025-11-13T03:07:17Z

PR Review: Add listing actors by only name to manager API

Overview

This PR adds a new listActors method to the manager driver interface that allows listing all actors by name, without requiring a specific key. This is a useful addition for querying actors when you know the actor type but not the specific key.

Code Quality - Positive

Strengths:

Clean interface definition with proper TypeScript typing
Consistent implementation across multiple drivers (FileSystem, Remote, Cloudflare, TestInline)
Follows existing patterns in the codebase
Good separation of concerns between driver implementations

Issues Found

1. Inconsistent parameter usage in ListActorsInput (router.ts:364-369)

The key and includeDestroyed parameters are passed to listActors but aren't used in any implementation
FileSystemManagerDriver only uses name, ignoring key and includeDestroyed
RemoteManagerDriver also only uses name
Recommendation: Either implement filtering by these parameters or remove them from the interface until needed

2. Logging inconsistency (cloudflare-workers/src/manager-driver.ts:352-358)

Message case should be all lowercase per CLAUDE.md conventions
Recommendation: Change to msg: listactors not fully implemented for cloudflare workers

3. Performance issue in FileSystemManagerDriver (drivers/file-system/manager.ts:337-345)

Iterates through ALL actors to find matches by name
No early termination or limit on results
For large actor counts, this could be slow and memory-intensive
Recommendation: Add pagination support (offset/limit parameters) and consider adding an index by actor name

4. No test coverage - CRITICAL

No tests added for the new listActors functionality
The PR modifies 9 files but doesn't include any test files
This is a significant gap for a new API endpoint
Recommendation: Add unit tests for each driver implementation and integration tests for the HTTP API

5. Cloudflare Workers implementation returns empty array

Currently logs a warning and returns empty array
This could be confusing for users who might think there are no actors
Recommendation: Consider throwing an Unsupported error instead

6. Missing pagination

The endpoint returns all matching actors without pagination
This could cause issues with large actor counts
Recommendation: Add cursor, limit, or offset/limit parameters

7. createTs is optional (manager/driver.ts:107)

Making timestamp optional could lead to inconsistent data
In router.ts:732, it falls back to Date.now() which might not be accurate
Recommendation: Make createTs required and ensure all implementations provide it

8. Unused key parameter in router.ts (line 367)

The key variable is passed to listActors but is guaranteed to be undefined at that point due to the else if check above
Recommendation: Remove the key parameter from the listActors call

Security Considerations

No authorization checks on the new endpoint
Users can list all actors of a given type across the system
Question: Is this intentional? Should there be namespace or permission filtering?
Potential for enumeration attacks without rate limiting

Summary

Overall Assessment: This is a solid addition that extends the manager API in a useful way. The implementation is clean and follows existing patterns.

Must Address:

Add comprehensive test coverage (critical)
Clarify/implement or remove unused parameters (key, includeDestroyed in ListActorsInput)
Add pagination support to prevent performance issues

Should Address:

Add validation for actor names
Improve Cloudflare Workers implementation or throw Unsupported error
Document security/authorization model
Make createTs required instead of optional

Great work overall! With test coverage and the pagination/parameter cleanup, this will be a solid feature.

claude · 2025-11-13T03:07:27Z

Code Review for PR #3430

Overview

This PR adds the ability to list actors by name only through the manager API. The implementation looks solid overall, but I've identified several areas that could benefit from improvements.

Critical Issues

1. Missing Implementation in Cloudflare Workers Driver

Location: rivetkit-typescript/packages/cloudflare-workers/src/manager-driver.ts:352-358

The Cloudflare Workers implementation returns an empty array with a warning. This creates an inconsistent API experience across different drivers.

Recommendation: Consider whether this functionality can be implemented using KV storage patterns similar to other methods in this driver, or if not possible, document this limitation clearly in the driver's interface or throw an explicit not implemented error rather than silently returning empty results.

Performance Concerns

2. Inefficient Full Table Scan in FileSystem Driver

Location: rivetkit-typescript/packages/rivetkit/src/drivers/file-system/manager.ts:337-359

This performs a full table scan filtering by name in memory. For large actor counts, this could become a performance bottleneck.

Recommendations:

Check if getActorsIterator supports filtering by name
Consider adding an index on actor names if this becomes a common query pattern
Add pagination support for the API to handle large result sets

Design and API Issues

3. Unused Parameters in ListActorsInput

Location: rivetkit-typescript/packages/rivetkit/src/manager/driver.ts:96-101

The key and includeDestroyed parameters are defined but never used in any implementation. Either implement these parameters or remove them from the interface. Unused parameters create confusion about API capabilities.

4. Inconsistent Optional createTs Field

Location: rivetkit-typescript/packages/rivetkit/src/manager/driver.ts:103-108

The createTs field is optional, but it's always populated in the FileSystem driver implementation and used in sorting. Either make this field required (preferred) or document when it may be undefined and ensure all code paths handle the undefined case correctly.

Code Quality Issues

5. Type Casting in FileSystem Driver

Location: rivetkit-typescript/packages/rivetkit/src/drivers/file-system/manager.ts:346

Using actor.key as string[] bypasses TypeScript's type safety. If actor.key is not actually a string[], this could cause runtime errors. Verify the actual type and either fix the type definition or add runtime validation.

6. Inconsistent Error Messages

Location: rivetkit-typescript/packages/rivetkit/src/manager/router.ts:282,292,357

The error messages have inconsistent styles. Standardize error message format for better developer experience.

Missing Test Coverage

7. No Tests for New Functionality

No test files were modified or added for the new listActors functionality.

Recommendation: Add tests covering:

Listing actors by name successfully
Empty results when no actors match
Sorting order validation (most recent first)
Error cases (missing name, invalid parameters)
Edge cases (no actors, many actors)

Consider adding tests in the driver test suite at rivetkit-typescript/packages/rivetkit/src/driver-test-suite/tests/

Security and Documentation

8. Consider Rate Limiting and Pagination

The listActors endpoint could return large result sets, which might consume excessive memory, create performance issues, or enable DoS vectors. Add pagination support (limit/offset or cursor-based) to the API.

9. Missing API Documentation

The new endpoint and driver method lack documentation comments explaining what actors are returned, sort order, parameter meanings, and return value constraints.

Summary

Strengths:

Clean integration across multiple driver implementations
Consistent interface addition to the ManagerDriver
Proper type exports through the module system

Required Changes:

Implement or properly document Cloudflare Workers limitation
Add test coverage for the new functionality
Address the unused parameters in ListActorsInput

Recommended Changes:
4. Optimize FileSystem driver implementation to avoid full table scans
5. Make createTs required or document when it can be undefined
6. Remove type casting or add validation
7. Add pagination support for scalability

Overall, this is a solid foundation for listing actors by name, but needs test coverage and consideration of the performance implications at scale.

claude · 2025-11-13T21:12:57Z

Code Review: Add listing actors by name to manager API

Summary

This PR adds a new listActors method to the manager API that allows listing all actors of a given name. The implementation looks solid overall, but there are a few areas that could be improved.

Issues and Concerns

1. Missing Implementation of `includeDestroyed` Parameter (Medium Priority)

The ListActorsInput interface defines an includeDestroyed?: boolean parameter, but none of the driver implementations actually use it:

File System Driver (drivers/file-system/manager.ts:337): Iterates through all actors but doesn't filter based on includeDestroyed
Remote Manager Driver (remote-manager-driver/mod.ts:260): Doesn't pass includeDestroyed to the API call
Cloudflare Workers Driver (cloudflare-workers/src/manager-driver.ts:352): Returns empty array (stub implementation)

Recommendation: Either implement the includeDestroyed filtering in all drivers, or remove the parameter from the interface if it's not needed yet.

2. Unused `key` Parameter in `ListActorsInput` (Low Priority)

The key?: string field in ListActorsInput (line 99 in manager/driver.ts) is passed through in the router (line 367) but never used by any driver implementation. This could be confusing.

Recommendation: Either implement key filtering or remove the parameter. If it's for future use, add a comment explaining this.

3. Inconsistent Type: `key` Parameter (Low Priority)

In ListActorsInput, the key parameter is typed as string (line 99), but other inputs like GetWithKeyInput use ActorKey (which appears to be string[]). The router passes key directly from the query string (line 367), which would be a string.

This is actually fine if the intent is to accept a single key component for filtering, but it's worth clarifying in a comment.

4. Performance Concern: File System Driver O(n) Iteration (Medium Priority)

The File System driver implementation (drivers/file-system/manager.ts:337-359) uses a full iteration through all actors:

for await (const actor of itr) {
    if (actor.name === name) {
        actors.push({...});
    }
}

Recommendation: If the state store supports indexed lookups by name, consider using that for better performance. If not, this is acceptable but might become slow with many actors.

5. Cloudflare Workers Stub Implementation (High Priority)

The Cloudflare Workers driver logs a warning and returns an empty array. This could silently fail in production.

Recommendation: Either:

Implement the actual functionality before merging
Throw an error if not implemented
Document this limitation clearly

Positive Observations

✅ Good error handling in the router with clear validation messages
✅ Proper sorting by creation timestamp (most recent first) in File System driver
✅ Consistent interface across all drivers
✅ Good use of structured logging in Remote Manager Driver
✅ Type safety maintained throughout the changes
✅ API endpoint properly implemented in remote-manager-driver

Minor Suggestions

Test Coverage: Consider adding tests for the new listActors functionality, particularly:
- Listing actors by name returns correct results
- Empty results when no actors match
- Sorting order is correct
- The includeDestroyed parameter works as expected (once implemented)
Documentation: Consider adding JSDoc comments to the listActors method and ListActorsInput interface to clarify:
- What the key parameter does (if anything)
- What includeDestroyed should do
- Expected return order (already sorted by creation time desc)
Consistency Check: The createTs field is optional in ActorOutput but always populated in the implementations. Consider making it required if it should always be present.

Security

No security concerns identified. The implementation properly validates inputs and uses existing authentication/authorization mechanisms.

Recommendations Before Merge

High Priority: Either implement or throw error in Cloudflare Workers driver
Medium Priority: Implement includeDestroyed filtering or remove the parameter
Low Priority: Add test coverage for the new functionality

Overall, this is a clean implementation that follows the existing patterns in the codebase well. The main concern is the incomplete implementation of declared parameters.

graphite-app · 2025-11-13T21:15:18Z

Merge activity

Nov 13, 9:15 PM UTC: NathanFlurry added this pull request to the Graphite merge queue.
Nov 13, 9:16 PM UTC: CI is running for this pull request on a draft pull request (#3465) due to your merge queue CI optimization settings.
Nov 13, 9:16 PM UTC: Merged by the Graphite merge queue via draft PR: #3465.

This was referenced Nov 10, 2025

chore(rivetkit): add WebSocketContext and RequestContext #3427

Closed

refactor(rivetkit): restructure connection lifecycle contexts #3428

Closed

refactor(rivetkit): extract WebSocket protocol parsing to shared utility #3429

Closed

vercel bot deployed to Preview – rivetkit-serverless November 10, 2025 01:13 View deployment

This was referenced Nov 13, 2025

feat(rivetkit): add ability to destroy actors #3458

Merged

wip: cf workers fix #3459

Closed

NathanFlurry force-pushed the 11-09-chore_rivetkit_add_listing_actors_by_only_name_to_manager_api branch from 02250ac to 51507e2 Compare November 13, 2025 02:47

NathanFlurry force-pushed the 11-09-refactor_rivetkit_extract_websocket_protocol_parsing_to_shared_utility branch from 8fdd032 to 14348ed Compare November 13, 2025 02:47

vercel bot deployed to Preview – rivetkit-serverless November 13, 2025 02:48 View deployment

This was referenced Nov 13, 2025

chore(rivetkit-typescript): remove dependency on node modules #3460

Merged

chore(rivetkit): switch dynamic node imports to use require #3461

Merged

chore(rivetkit): fix type checks #3462

Merged

This was referenced Nov 13, 2025

feat(gas): implement smart pulling for horizontally scaled workers #3417

Merged

fix(guard): handle websocket tasks during shutdown #3448

Merged

fix: improve sigterm handling for the entire runtime #3443

Merged

NathanFlurry mentioned this pull request Nov 13, 2025

feat(gateway): websocket hibernation #3400

Merged

This was referenced Nov 13, 2025

fix(pb): dont immediately reallocate actors on runner stopping #3413

Merged

fix(guard): dont send open msg for hibernating ws, hibernating ws keepalive #3449

Merged

NathanFlurry mentioned this pull request Nov 13, 2025

chore(gas): rename wake sub to bump sub #3408

Merged

MasterPtato approved these changes Nov 13, 2025

View reviewed changes

chore(rivetkit): add listing actors by only name to manager api

a8f3a9c

NathanFlurry force-pushed the 11-09-chore_rivetkit_add_listing_actors_by_only_name_to_manager_api branch from 51507e2 to a8f3a9c Compare November 13, 2025 20:53

vercel bot deployed to Preview – rivetkit-serverless November 13, 2025 20:55 View deployment

NathanFlurry mentioned this pull request Nov 13, 2025

chore(rivetkit): implement new hibernating ws protocol #3464

Open

graphite-app bot pushed a commit that referenced this pull request Nov 13, 2025

chore(rivetkit): add listing actors by only name to manager api (#3430)

cd22f31

graphite-app bot closed this Nov 13, 2025

graphite-app bot deleted the 11-09-chore_rivetkit_add_listing_actors_by_only_name_to_manager_api branch November 13, 2025 21:16

NathanFlurry mentioned this pull request Nov 13, 2025

feat(cloudflare-workers): add createInlineClient #3466

Open

NicholasKissel mentioned this pull request Nov 15, 2025

chre: v4 website changes #3472

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(rivetkit): add listing actors by only name to manager api #3430

chore(rivetkit): add listing actors by only name to manager api #3430

Uh oh!

NathanFlurry commented Nov 10, 2025

Uh oh!

vercel bot commented Nov 10, 2025 •

edited

Loading

Uh oh!

NathanFlurry commented Nov 10, 2025 •

edited by jog1t

Loading

Uh oh!

pkg-pr-new bot commented Nov 10, 2025 •

edited

Loading

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

claude bot commented Nov 13, 2025

Uh oh!

graphite-app bot commented Nov 13, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chore(rivetkit): add listing actors by only name to manager api #3430

chore(rivetkit): add listing actors by only name to manager api #3430

Uh oh!

Conversation

NathanFlurry commented Nov 10, 2025

Uh oh!

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

Uh oh!

Uh oh!

NathanFlurry commented Nov 10, 2025 • edited by jog1t 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 Nov 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Nov 13, 2025

Code Review

✅ Positive Aspects

🔍 Issues & Concerns

1. Missing Test Coverage (High Priority)

2. Incomplete Cloudflare Workers Implementation (Medium Priority)

3. Unused Parameters in ListActorsInput (Low Priority)

4. Potential Performance Issue (Medium Priority)

5. Missing Optional Timestamp (Low Priority)

6. Inconsistent Error Handling (Low Priority)

📝 Minor Suggestions

🔒 Security

🎯 Summary

Uh oh!

claude bot commented Nov 13, 2025

PR Review: Add listing actors by only name to manager API

Overview

Code Quality - Positive

Issues Found

Security Considerations

Summary

Uh oh!

claude bot commented Nov 13, 2025

Code Review for PR #3430

Overview

Critical Issues

1. Missing Implementation in Cloudflare Workers Driver

Performance Concerns

2. Inefficient Full Table Scan in FileSystem Driver

Design and API Issues

3. Unused Parameters in ListActorsInput

4. Inconsistent Optional createTs Field

Code Quality Issues

5. Type Casting in FileSystem Driver

6. Inconsistent Error Messages

Missing Test Coverage

7. No Tests for New Functionality

Security and Documentation

8. Consider Rate Limiting and Pagination

9. Missing API Documentation

Summary

Uh oh!

claude bot commented Nov 13, 2025

Code Review: Add listing actors by name to manager API

Summary

Issues and Concerns

1. Missing Implementation of includeDestroyed Parameter (Medium Priority)

2. Unused key Parameter in ListActorsInput (Low Priority)

3. Inconsistent Type: key Parameter (Low Priority)

4. Performance Concern: File System Driver O(n) Iteration (Medium Priority)

5. Cloudflare Workers Stub Implementation (High Priority)

Positive Observations

Minor Suggestions

Security

Recommendations Before Merge

Uh oh!

graphite-app bot commented Nov 13, 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

vercel bot commented Nov 10, 2025 •

edited

Loading

NathanFlurry commented Nov 10, 2025 •

edited by jog1t

Loading

pkg-pr-new bot commented Nov 10, 2025 •

edited

Loading

1. Missing Implementation of `includeDestroyed` Parameter (Medium Priority)

2. Unused `key` Parameter in `ListActorsInput` (Low Priority)

3. Inconsistent Type: `key` Parameter (Low Priority)

graphite-app bot commented Nov 13, 2025 •

edited

Loading