fix(namespace): bring namespace_list api calls up to date

[abcxff]

Fixes rivet-engine on multiple datacenters

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Ready	Preview	Comment	Nov 11, 2025 0:02am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Nov 11, 2025 0:02am
rivet-inspector	Ignored	Preview		Nov 11, 2025 0:02am
rivet-site	Ignored	Preview		Nov 11, 2025 0:02am

[abcxff]

• fix(namespace): bring namespace_list api calls up to date #3444 👈 (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: fix(namespace): bring namespace_list api calls up to date

Thank you for fixing this synchronization issue between the API endpoint and the operation! The changes correctly align the query parameter serialization format. Here's my detailed review:

✅ Positive Aspects

1. Correct Fix: The change from multiple query parameters to a single comma-separated parameter is the right approach and matches the pattern used elsewhere in the codebase (e.g., actor_ids in api-peer/src/actors/list.rs:17-20).

2. Good Documentation: The added NOTE comments linking the two files together are excellent for maintainability (get_global.rs:30 and namespaces.rs:9).

3. Consistency: The serialization format (comma-separated IDs) matches the deserialization format used in the API endpoint (namespaces.rs:12-16).

4. Clean Implementation: Using .join(",") is idiomatic and matches patterns in api-public/src/actors/utils.rs:84-87.

🔍 Observations & Suggestions

1. Minor: Visibility Change (namespaces.rs:65-67)

The change from private to public fields in CreateRequest:

pub name: String,
pub display_name: String,

This wasn't mentioned in the PR description. While it doesn't affect the fix, I'm curious if this was intentional or needed for another purpose?

2. Testing Consideration

I couldn't find existing tests for the namespace list functionality. While not a blocker for this PR, consider adding integration tests similar to engine/packages/engine/tests/actors_list.rs to prevent future regressions.

Example test scenario:

• Create multiple namespaces
• Call the peer API endpoint with comma-separated namespace_ids
• Verify all namespaces are returned correctly

3. Edge Case: Empty IDs

The current implementation handles empty namespace_ids well (returns empty string which should work), but consider what happens if:

• namespace_ids is an empty vector
• All IDs fail to parse/convert to string

The code appears safe, but explicit handling might improve robustness.

4. Code Style (Per CLAUDE.md)

✅ Follows structured logging conventions
✅ Uses lowercase log messages
✅ Proper use of tracing

🎯 Summary

This is a solid fix that correctly addresses the API synchronization issue. The core change is correct and follows established patterns in the codebase.

Recommendation: ✅ APPROVE with minor suggestions for future improvement.

The fix can be merged as-is, with testing and the visibility change being minor follow-up considerations rather than blockers.

---

Generated with Claude Code - Automated PR Review

[pkg-pr-new]

More templates

• example-ai-agent
• example-background-jobs
• example-better-auth-external-db
• example-bots
• chat-room
• example-cloudflare-workers
• example-cloudflare-workers-hono
• example-counter
• example-counter-serverless
• example-crdt
• example-cursors
• example-cursors-raw-websocket
• example-database
• example-deno
• example-sqlite
• example-elysia
• example-express
• example-freestyle
• example-game
• example-hono
• example-hono-bun
• example-hono-react
• example-kitchen-sink
• example-next-js
• example-rate
• example-raw-fetch-handler
• example-raw-websocket-handler
• example-raw-websocket-handler-proxy
• example-react
• starter
• example-stream
• example-sync
• example-tenant
• example-trpc
• example-workflows

@rivetkit/actor

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

@rivetkit/cloudflare-workers

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

@rivetkit/core

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

@rivetkit/db

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

@rivetkit/framework-base

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

@rivetkit/next-js

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

@rivetkit/react

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

rivetkit

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

@rivetkit/sql-loader

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

@rivetkit/engine-runner

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

@rivetkit/engine-runner-protocol

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

commit: db0d75d

[claude]

PR Review: Fix namespace_list API calls

Summary

This PR updates two namespace operations (get_global and resolve_for_name_global) to properly use the typed ListQuery struct instead of raw query parameters. This is a good improvement that adds type safety and ensures consistency with the API contract.

✅ Strengths

1. Type Safety: Replacing raw .query() calls with the typed ListQuery struct prevents runtime errors from API contract mismatches
2. API Contract Compliance: The changes ensure the peer API calls match the expected query parameter structure defined in rivet_api_types::namespaces::list::ListQuery
3. Proper Use of Existing Client: Good use of rivet_pools::reqwest::client().await? per CLAUDE.md guidelines
4. Minimal Surface Area: Focused change that addresses a specific issue without unnecessary modifications

🐛 Potential Issue

Variable Name Bug in resolve_for_name_global.rs:40

Line 40 uses input.name instead of the closure parameter key:

name: Some(input.name),  // ❌ Using input.name

This should likely be:

name: Some(key),  // ✅ Using the closure parameter

The closure receives key as a parameter (line 29: move |mut cache, key|), which should be the same as input.name, but using input.name creates an unnecessary capture. While this may work functionally (since input.name is cloned earlier on line 26), it's inconsistent with the closure's design pattern and could cause issues if the code is refactored.

📝 Minor Observations

1. API Peer Struct Fields: Making CreateRequest fields public is correct if they need external access, though the reason for this change isn't clear from the PR description
2. Version Bump: The openapi.json version bump to 2.0.24-rc.1 is appropriate for an API contract change
3. Missing Tests: No test coverage added for these changes. Consider adding integration tests to verify the peer API calls work correctly

🎯 Recommendations

1. Fix the variable name issue in resolve_for_name_global.rs:40 to use key instead of input.name
2. Add test coverage for the peer API calls, especially testing the query parameter serialization
3. Update PR description to explain why CreateRequest fields were made public

Security & Performance

• ✅ No security concerns identified
• ✅ Performance impact is neutral (just changing the query construction method)
• ✅ Proper use of connection pooling via rivet_pools::reqwest::client()

Overall, this is a solid improvement that adds type safety. The main issue to address is the variable name inconsistency in the closure.

[claude]

Code Review

Summary

This PR updates namespace API calls to use the structured ListQuery type instead of raw query parameter tuples. This is a good refactoring that improves type safety and consistency with the rest of the codebase.

Positive Aspects ✅

1. Type Safety Improvement: The change from raw query tuples to the structured ListQuery type is excellent. This provides better type checking at compile time and aligns with how similar operations work elsewhere in the codebase (e.g., rivet_api_types::actors::list::ListQuery in pegboard/src/ops/actor/get_for_key.rs:76-84).

2. Consistency: The implementation now matches the API endpoint handler pattern in api-public/src/namespaces.rs:38-44, which also uses the ListQuery struct for remote datacenter requests.

3. Correct String Format: The namespace_ids field is properly formatted as a comma-separated string (line 39 in get_global.rs), which matches the expected format in the peer API handler (api-peer/src/namespaces.rs:11-15) that parses comma-separated IDs.

4. Complete Implementation: Both get_global.rs and resolve_for_name_global.rs were updated consistently.

Issues & Suggestions 🔍

Critical Issue: Variable Shadowing Bug in resolve_for_name_global.rs

Location: engine/packages/namespace/src/ops/resolve_for_name_global.rs:40

.query(rivet_api_types::namespaces::list::ListQuery {
    namespace_ids: None,
    limit: None,
    cursor: None,
    name: Some(input.name),  // ❌ Should be input.name.clone()
})

Problem: The code uses input.name directly instead of input.name.clone(). This variable is captured by the closure at line 26 (move |mut cache, key|) and then accessed again at line 40. However, since input is moved into the closure, you're accessing a field from a moved value.

Expected Behavior: This should fail to compile with an error like "use of moved value" or "borrow of moved value". The fact that this compiles suggests either:

1. There's an implicit clone happening somewhere
2. The code hasn't been tested/compiled yet

Fix: Change line 40 to:

name: Some(input.name.clone()),

This matches the pattern used elsewhere in the function and in get_global.rs.

Minor: Unused Variable

Location: engine/packages/namespace/src/ops/resolve_for_name_global.rs:29

The closure parameter key is captured but appears to be the same as input.name. Consider using key consistently instead of input.name within the closure body for clarity, or clarify why both are needed.

Testing Recommendations 🧪

1. Compile Test: Verify the code compiles without warnings, especially around the variable capture issue mentioned above.
2. Integration Test: Test both operations with:
   • Single namespace ID lookup
   • Multiple namespace IDs
   • Name-based resolution
   • Non-existent namespace/name queries
3. Cross-Datacenter Test: Verify the remote datacenter request path works correctly with the new query structure.

Performance & Security ✅

• Performance: No performance concerns. The structured query type doesn't add meaningful overhead.
• Security: The change maintains proper input handling. The UUID parsing in the peer handler (api-peer/src/namespaces.rs:13) will safely filter invalid IDs.

Overall Assessment

This is a solid refactoring that improves code quality, but there's a critical bug that needs to be fixed in resolve_for_name_global.rs before merging. Once the variable shadowing issue is resolved, this PR will be ready to merge.

Recommendation: Request changes to fix the input.name vs input.name.clone() issue.