fix: api changes

[MasterPtato]

Changes

[MasterPtato]

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

• fix: get pb manager test working e2e again #2721
• fix: convert runner protocol to protobuf #2702
• fix: even more actor api changes #2714
• chore: flatten actor api spec #2698
• fix: increase default actor kill timeout to 5s #2701
• fix: dont clear ingress ports on reschedule #2700
• fix: add cpu constraints to oci config #2697
• fix: api changes #2692 👈 (View in Graphite)
• fix: fix logs for new actors #2649
• feat: add actor queue #2644
• feat: add actor kv to runners #2638
• feat: get multi actors working e2e on docker compose #2637
• fix: add udp ports to docker compose #2635
• chore: fix merge conflicts #2634
• fix: get new actors working on docker compose #2623
• fix: figure out actor log shipping #2622
• fix(pegboard): convert ws manager socket to unix socket #2478
• fix(pb): get new actor ids working e2e #2471
• feat: implement new actor ids #2467
• feat(pb): get multi-actors working e2e, add resources for builds #2465
• feat(pb): add runner logs #2447
• feat: add runners to pb protocol #2433
• chore(pegboard): add exit code log from container runner #2477
• fix(workflows): fix branch version bug #2475
• chore(pegboard): replace computing image size manually with using tar bytes read #2625
• fix: make pb image downloads random, remove prewarm from pb protocol #2784
• fix: clean up worker threads in the event of an error #2783
• fix: make query_as macros retry #2773
• 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]

Claude finished @MasterPtato's task —— View job

---

PR Review Complete

I've thoroughly reviewed this pull request that implements API versioning changes for the actor/container functionality. Here's my comprehensive analysis:

Todo List

• Analyze changed files and understand the scope of API changes
• Review Rust macro and API route changes
• Review Fern API definition changes
• Identify potential issues and provide feedback
• Post comprehensive review findings

---

Summary of Changes

This PR implements API versioning by:

1. Creating v1 versioned actor APIs (actors/v1/) that match the original v2 structure but with internal: true
2. Adding container APIs (containers/) that mirror the actor functionality
3. Updating Rust macro and route definitions to support the new structure
4. Maintaining backward compatibility through route mounting

---

Code Quality & Best Practices ✅

Strengths:

• Consistent API Design: Container APIs follow the same patterns as actor APIs
• Proper Versioning Strategy: Clear separation between v1 (internal) and v2 (public) APIs
• Documentation: Good inline documentation in YAML files
• Backward Compatibility: Existing routes maintained through mounts

Areas for Improvement:

• Code Duplication: Significant duplication between actor and container definitions
• Missing Validation: Some optional parameters lack validation constraints

---

Potential Issues 🔍

1. Code Duplication Concern (packages/core/api/actor/src/route/mod.rs:243-252)

mounts: [
    {
        path: OldRouter,
    },
    {
        path: OldRouter,
        prefix: "/v1"
    },
],

The same OldRouter is mounted twice - once without prefix and once with /v1. This could lead to conflicting routes.

2. API Consistency Issues

• Base Path Differences: v1 actors use /actors while v2 uses /v2/actors, containers use /v1/containers
• Internal vs Public: v1 actors marked as internal: true but containers are not

3. Missing Rate Limiting (sdks/api/fern/definition/actors/v1/logs.yml:12-30)

The v1 actor logs endpoint lacks rate limiting compared to the v2 version in the route definitions.

4. Parameter Naming Inconsistency

• Actor logs use actor_ids_json
• Container logs use container_ids_json
• Both should follow the same pattern for consistency

---

Performance Considerations ⚡

Good:

• Efficient Route Mounting: Proper use of prefix-based routing
• Proper Rate Limiting: Most endpoints have appropriate rate limits

Concerns:

• Multiple Route Evaluation: The dual mounting of OldRouter may cause unnecessary route evaluation overhead

---

Security Assessment 🔒

Strengths:

• Authentication Required: All endpoints properly require authentication
• Rate Limiting: Most endpoints have appropriate rate limiting
• Input Validation: Query parameters are properly typed

Recommendations:

• Consistent Internal Marking: Consider marking container APIs as internal if they should be
• Audit Access Patterns: Ensure v1 vs v2 access controls are intentional

---

Test Coverage 🧪

Limitation: Cannot assess test coverage as tests are not included in this PR.

Recommendations:

• Ensure integration tests cover the new versioned endpoints
• Test backward compatibility with existing clients
• Verify rate limiting works correctly on new endpoints
• Test container vs actor API consistency

---

Specific Recommendations

High Priority

1. Resolve Route Conflicts: packages/core/api/actor/src/route/mod.rs:243-252

   • Consider if both mounts of OldRouter are necessary
   • Document the routing precedence clearly

2. Add Missing Rate Limiting: Apply consistent rate limiting to v1 actor logs endpoint

Medium Priority

3. Reduce Code Duplication: Consider extracting common API patterns into shared modules
4. Standardize Naming: Use consistent parameter naming across actor/container APIs
5. Clarify API Versioning Strategy: Document when to use v1 vs v2 vs container APIs

Low Priority

6. Documentation: Add API migration guide for developers
7. Monitoring: Consider adding metrics to track v1 vs v2 API usage

---

Overall Assessment

This is a well-structured API versioning implementation that maintains backward compatibility while introducing container functionality. The code follows established patterns and includes proper authentication and rate limiting.

Approval Status: ✅ Approved with minor concerns

The main issues are around route conflicts and missing rate limiting, which should be addressed but don't block the PR.

---

[cloudflare-workers-and-pages]

Deploying rivet with    Cloudflare Pages

Latest commit:	f4ea5e9
Status:	🚫  Build failed.

View logs

[claude]

Claude finished @MasterPtato's task —— View job

---

PR Review Complete

---

Todo List

• Analyze changed files and understand the scope of API changes
• Review Rust macro and API route changes
• Review Fern API definition changes
• Identify potential issues and provide feedback
• Post comprehensive review findings

---

Summary of Changes

This PR implements API versioning by:

1. Creating v1 versioned actor APIs (actors/v1/) that match the original v2 structure but with internal: true
2. Adding container APIs (containers/) that mirror the actor functionality
3. Updating Rust macro and route definitions to support the new structure
4. Maintaining backward compatibility through route mounting

---

Code Quality & Best Practices ✅

Strengths:

• Consistent API Design: Container APIs follow the same patterns as actor APIs
• Proper Versioning Strategy: Clear separation between v1 (internal) and v2 (public) APIs
• Documentation: Good inline documentation in YAML files
• Backward Compatibility: Existing routes maintained through mounts

Areas for Improvement:

• Code Duplication: Significant duplication between actor and container definitions
• Missing Validation: Some optional parameters lack validation constraints

---

Potential Issues 🔍

1. Code Duplication Concern (packages/core/api/actor/src/route/mod.rs:243-252)

mounts: [
    {
        path: OldRouter,
    },
    {
        path: OldRouter,
        prefix: "/v1"
    },
],

The same OldRouter is mounted twice - once without prefix and once with /v1. This could lead to conflicting routes.

2. API Consistency Issues

• Base Path Differences: v1 actors use /actors while v2 uses /v2/actors, containers use /v1/containers
• Internal vs Public: v1 actors marked as internal: true but containers are not

3. Missing Rate Limiting (sdks/api/fern/definition/actors/v1/logs.yml:12-30)

The v1 actor logs endpoint lacks rate limiting compared to the v2 version in the route definitions.

4. Parameter Naming Inconsistency

• Actor logs use actor_ids_json
• Container logs use container_ids_json
• Both should follow the same pattern for consistency

---

Performance Considerations ⚡

Good:

• Efficient Route Mounting: Proper use of prefix-based routing
• Proper Rate Limiting: Most endpoints have appropriate rate limits

Concerns:

• Multiple Route Evaluation: The dual mounting of OldRouter may cause unnecessary route evaluation overhead

---

Security Assessment 🔒

Strengths:

• Authentication Required: All endpoints properly require authentication
• Rate Limiting: Most endpoints have appropriate rate limiting
• Input Validation: Query parameters are properly typed

Recommendations:

• Consistent Internal Marking: Consider marking container APIs as internal if they should be
• Audit Access Patterns: Ensure v1 vs v2 access controls are intentional

---

Test Coverage 🧪

Limitation: Cannot assess test coverage as tests are not included in this PR.

Recommendations:

• Ensure integration tests cover the new versioned endpoints
• Test backward compatibility with existing clients
• Verify rate limiting works correctly on new endpoints
• Test container vs actor API consistency

---

Specific Recommendations

High Priority

1. Resolve Route Conflicts: packages/core/api/actor/src/route/mod.rs:243-252

   • Consider if both mounts of OldRouter are necessary
   • Document the routing precedence clearly

2. Add Missing Rate Limiting: Apply consistent rate limiting to v1 actor logs endpoint

Medium Priority

3. Reduce Code Duplication: Consider extracting common API patterns into shared modules
4. Standardize Naming: Use consistent parameter naming across actor/container APIs
5. Clarify API Versioning Strategy: Document when to use v1 vs v2 vs container APIs

Low Priority

6. Documentation: Add API migration guide for developers
7. Monitoring: Consider adding metrics to track v1 vs v2 API usage

---

Overall Assessment

This is a well-structured API versioning implementation that maintains backward compatibility while introducing container functionality. The code follows established patterns and includes proper authentication and rate limiting.

Approval Status: ✅ Approved with minor concerns

The main issues are around route conflicts and missing rate limiting, which should be addressed but don't block the PR.

---

[claude]

Claude finished @MasterPtato's task —— View job

---

---

🔍 PR Review Complete

---

Todo List

• Understand the scope of API changes across 100 files
• Review Rust route structure and API organization
• Analyze Fern API definitions and versioning strategy
• Check for code quality issues and best practices
• Identify potential bugs and security concerns
• Assess performance implications
• Evaluate test coverage needs
• Provide comprehensive feedback with file references

---

📋 Summary of Changes

This PR implements major API versioning by:

1. Creating v2 actors API (/v2/actors) - new public API with proper versioning
2. Preserving v1 actors API (/actors, /v1/actors) - deprecated, internal-only via route mounting
3. Adding v1 containers API (/v1/containers) - new runtime API mirroring actor functionality
4. Maintaining backward compatibility through strategic route mounting

Scale: 39,815 additions, 5,038 deletions across 100 files

---

✅ Code Quality & Best Practices

Strengths:

• Consistent Authentication: All endpoints properly require authentication
• Proper Error Handling: Good use of error types and validation
• Documentation: Well-documented API endpoints in YAML files
• Resource Management: Appropriate use of connection pooling and caching
• Tracing: Good instrumentation with #[tracing::instrument]

Areas for Improvement:

• Code Duplication: Significant duplication between actor and container implementations
• Missing Documentation: Some complex route mounting logic lacks explanation

---

🚨 Critical Issues

1. Route Conflict Risk

Location: packages/core/api/actor/src/route/mod.rs:333-338

mounts: [
    {
        path: OldRouter,
    },
    {
        path: OldRouter,
        prefix: "/v1"
    },
],

Issue: OldRouter is mounted twice - once without prefix and once with /v1 prefix. This could cause routing conflicts and undefined behavior.

Recommendation: Clarify the intended routing precedence or consolidate the mounting strategy.

2. Missing Rate Limiting

Location: packages/core/api/actor/src/route/mod.rs:415-420

"actors" / "logs": {
    GET: actors::logs::get_logs(
        query: actors::logs::GetActorLogsQuery,
        opt_auth: true,
    ),
},

Issue: The v1 actor logs endpoint in OldRouter lacks rate limiting, while the v2 version has proper rate limits.

Recommendation: Add consistent rate limiting across all log endpoints.

3. API Inconsistencies

Parameter Naming:

• Actor logs: actor_ids_json
• Container logs: container_ids_json
• Recommendation: Use consistent naming patterns

ID Types:

• Actors v1: uuid in path parameters
• Containers: commons.Id in path parameters
• Recommendation: Standardize ID types across APIs

4. Response Structure Differences

Location: sdks/api/fern/definition/actors/v1/logs.yml:36-55 vs sdks/api/fern/definition/containers/logs.yml:35-57

Issue: Container logs include foreigns field but actor v1 logs don't, creating API inconsistency.

---

🛡️ Security Assessment

Strengths:

• Authentication Required: All endpoints properly require authentication
• Input Validation: Proper validation of actor/container IDs and parameters
• Authorization Checks: Appropriate game/environment permission checks
• Rate Limiting: Most endpoints have proper rate limiting

Concerns:

• Inconsistent Rate Limiting: Some v1 endpoints lack rate limiting protection
• Cache Security: Actor validation caching appears secure but should be audited for race conditions

Recommendations:

• Ensure consistent rate limiting across all API versions
• Audit caching mechanisms for potential security issues

---

⚡ Performance Considerations

Good Practices:

• Efficient Caching: Actor validation uses Redis caching with proper key management
• Connection Reuse: Appropriate use of connection pooling
• Pagination: Proper pagination support in list endpoints

Potential Issues:

• Dual Route Evaluation: The dual mounting of OldRouter may cause unnecessary route evaluation overhead
• Code Duplication: Duplicated handlers increase binary size and maintenance overhead

Recommendations:

• Resolve the dual route mounting issue
• Consider extracting common functionality to reduce duplication

---

🧪 Test Coverage

Limitations: Cannot directly assess test coverage as tests are not included in this PR.

Critical Testing Areas:

1. API Versioning: Ensure v1 and v2 endpoints work correctly
2. Route Precedence: Test that route mounting doesn't cause conflicts
3. Rate Limiting: Verify rate limits work on all endpoints
4. Backward Compatibility: Ensure existing clients continue working
5. Container vs Actor Parity: Test that containers and actors behave consistently

Recommendations:

• Add integration tests for the new versioned endpoints
• Test edge cases in route resolution
• Verify rate limiting behavior across all API versions
• Add performance tests for the new endpoints

---

📝 Detailed Recommendations

High Priority

1. Fix Route Conflicts: Resolve the dual OldRouter mounting issue
2. Add Missing Rate Limiting: Apply consistent rate limiting to v1 actor logs
3. Standardize API Patterns: Align parameter naming and response structures

Medium Priority

4. Reduce Code Duplication: Extract common patterns into shared modules
5. Improve Documentation: Add comments explaining the route mounting strategy
6. API Consistency: Standardize ID types and response formats across APIs

Low Priority

7. Performance Optimization: Consider optimizing route evaluation
8. Monitoring: Add metrics to track v1 vs v2 API usage
9. Migration Guide: Create documentation for API migration

---

🎯 Overall Assessment

This is a well-architected API versioning implementation that maintains backward compatibility while introducing new functionality. The code follows established patterns and includes proper authentication and validation.

Approval Status: ✅ Approved with conditions

The main concerns are around route conflicts and missing rate limiting, which should be addressed but don't fundamentally block the PR.

Confidence: High - thorough analysis of core routing, security, and API design patterns

---

[claude]

Claude finished @MasterPtato's task —— View job

---

PR Review Complete

Todo List

• Analyze scope and recent changes since last review
• Review API structure and route definitions
• Check code quality and best practices
• Identify potential bugs and security issues
• Assess performance implications
• Evaluate test coverage considerations
• Provide actionable feedback with specific references

---

Summary of Changes

This PR implements major API versioning by introducing:

• V1 Actor APIs (/actors) marked as deprecated/internal
• V1 Container APIs (/v1/containers) that mirror actor functionality
• V2 Actor APIs (/v2/actors) as the new public interface
• Backward compatibility through route mounting and deprecated endpoints

Critical Security Issues 🚨

1. Missing Rate Limiting on All Log Endpoints (packages/core/api/actor/src/route/mod.rs)

CRITICAL: All log endpoints across all API versions lack rate limiting:

• V2 actor logs (line 120-125)
• V1 container logs (line 209-214)
• OldRouter actor logs (line 415-420)
• Deprecated V1 logs (line 582-586)

Impact: Potential DoS attacks, resource exhaustion, uncontrolled access to potentially expensive log operations.

Recommendation: Add rate limiting consistent with metrics endpoints:

rate_limit: {
    buckets: [
        { count: 100, bucket: duration::minutes(1) },
    ],
},

2. Deprecated Endpoint Lacks Authentication (packages/core/api/actor/src/route/mod.rs:582-586)

The deprecated V1 logs endpoint is missing opt_auth: true.

---

Code Quality Issues 🔍

3. Route Mounting Conflicts (packages/core/api/actor/src/route/mod.rs:331-339)

mounts: [
    {
        path: OldRouter,
    },
    {
        path: OldRouter,
        prefix: "/v1"
    },
],

The same OldRouter is mounted twice, potentially causing routing conflicts or unnecessary overhead.

4. API Inconsistency: ID Types

• V1 actors: Use uuid type (sdks/api/fern/definition/actors/v1/package.yml:21,67)
• V2 actors: Use commons.Id type (sdks/api/fern/definition/actors/package.yml:20,66)
• V1 containers: Use commons.Id type (sdks/api/fern/definition/containers/package.yml:20,66)

This creates inconsistent client experiences across API versions.

5. Semantic Mismatch: Container Logs Using Actor Operations (packages/core/api/actor/src/route/containers/logs.rs:102,142)

Container logs use pegboard::ops::actor::log::read::Input instead of container-specific operations:

ctx.op(pegboard::ops::actor::log::read::Input {
    actor_ids: container_ids_clone.clone(),  // using actor_ids for containers!

6. Variable Naming Inconsistencies (packages/core/api/actor/src/route/containers/logs.rs:162-164)

Container logs code uses actor_id field names when processing container data:

if !container_id_to_index.contains_key(&entry.actor_id) {
    container_id_to_index.insert(entry.actor_id.clone(), ...);
    unique_container_ids.push(entry.actor_id.to_string());  

---

Performance Concerns ⚡

7. Massive Code Duplication

Near-identical implementations across:

• Actor logs (v1 vs v2): ~240 lines duplicated
• Container vs Actor APIs: ~95% code similarity
• Multiple route definitions with same logic

Impact: Increased binary size, maintenance burden, potential for divergent behavior.

8. Inefficient Route Evaluation

The dual mounting of OldRouter may cause unnecessary route evaluation overhead for every request.

---

Security Assessment 🔒

Strengths:

• Proper authentication required on most endpoints
• Input validation through typed parameters
• Use of JSON parsing with error handling

Critical Gaps:

• No rate limiting on log endpoints (most critical)
• Missing authentication on deprecated endpoint
• No input size limits on *_ids_json parameters

---

API Design Issues 📐

9. Inconsistent Deprecation Strategy

• V1 actors marked as availability: deprecated (line 8) and audiences: internal (line 12)
• V1 containers have audiences: runtime (line 11) but no deprecation marking
• Inconsistent base paths: /actors vs /v2/actors vs /v1/containers

10. Missing Parameter Validation

JSON parameters like actor_ids_json and container_ids_json lack size limits, potentially allowing large payload attacks.

---

Test Coverage Concerns 🧪

Cannot assess as tests are not included in this PR. Critical areas needing coverage:

• Rate limiting behavior (especially missing coverage for log endpoints)
• Route conflict resolution with dual mounting
• API consistency between actor/container endpoints
• Backward compatibility with deprecated endpoints

---

Specific Recommendations

High Priority (Security)

1. Add rate limiting to ALL log endpoints - Critical security gap
2. Fix deprecated endpoint authentication (line 582-586)
3. Add input size limits on JSON parameters

Medium Priority (Correctness)

4. Resolve route mounting conflicts (lines 331-339)
5. Fix semantic mismatch in container logs using actor operations
6. Standardize ID types across API versions
7. Fix variable naming in container logs implementation

Low Priority (Maintenance)

8. Reduce code duplication through shared modules
9. Document API versioning strategy and migration paths
10. Add monitoring for v1 vs v2 API usage patterns

---

Overall Assessment

Status: ❌ Major Issues Require Resolution

While this PR successfully implements API versioning with backward compatibility, it introduces critical security vulnerabilities through missing rate limiting on log endpoints. The implementation also suffers from significant code duplication and API inconsistencies that will impact maintainability.

Priority: Fix security issues before merge, address correctness issues in follow-up PRs.

---

[claude]

Claude finished @MasterPtato's task —— View job

---

🔍 PR Review Complete

Todo List

• Analyze the scope and structure of API versioning changes
• Review core routing and API handler implementations
• Check security aspects (authentication, rate limiting, input validation)
• Examine code quality and architectural decisions
• Identify potential bugs and edge cases
• Assess performance implications
• Evaluate test coverage considerations
• Provide comprehensive feedback with specific file references

---

📋 Summary of Changes

This PR implements major API versioning by:

1. Creating v2 actors API (/v2/actors) - new public API with proper versioning
2. Preserving v1 actors API (/actors, /v1/actors) - deprecated, internal-only via route mounting
3. Adding v1 containers API (/v1/containers) - new runtime API mirroring actor functionality
4. Maintaining backward compatibility through strategic route mounting

Scale: 39,815 additions, 5,038 deletions across 100 files

---

🚨 Critical Security Issues

1. Missing Rate Limiting on ALL Log Endpoints

CRITICAL: All log endpoints across all API versions lack rate limiting:

• V2 actor logs (packages/core/api/actor/src/route/mod.rs:120-125)
• V1 container logs (packages/core/api/actor/src/route/mod.rs:209-214)
• OldRouter actor logs (packages/core/api/actor/src/route/mod.rs:415-420)
• Deprecated V1 logs (packages/core/api/actor/src/route/mod.rs:582-586)

Impact: Potential DoS attacks, resource exhaustion, uncontrolled access to potentially expensive log operations.

Recommendation: Add rate limiting consistent with metrics endpoints:

rate_limit: {
    buckets: [
        { count: 100, bucket: duration::minutes(1) },
    ],
},

2. Deprecated Endpoint Lacks Authentication

Location: packages/core/api/actor/src/route/mod.rs:582-586
The deprecated V1 logs endpoint is missing opt_auth: true.

---

🔍 Code Quality & Architecture Issues

3. Route Mounting Conflicts

Location: packages/core/api/actor/src/route/mod.rs:331-339 and packages/edge/api/actor/src/route/mod.rs:188-196

mounts: [
    {
        path: OldRouter,
    },
    {
        path: OldRouter,
        prefix: "/v1"
    },
],

The same OldRouter is mounted twice, potentially causing routing conflicts or unnecessary overhead.

4. API Inconsistency: ID Types

• V1 actors: Use uuid type (sdks/api/fern/definition/actors/v1/__package__.yml:21,67)
• V2 actors: Use commons.Id type (packages/core/api/actor/src/route/actors/logs.rs:51)
• V1 containers: Use commons.Id type (sdks/api/fern/definition/containers/__package__.yml:20,66)

This creates inconsistent client experiences across API versions.

5. Semantic Mismatch: Container Logs Using Actor Operations

Location: packages/core/api/actor/src/route/containers/logs.rs:101,142
Container logs use pegboard::ops::actor::log::read::Input instead of container-specific operations:

ctx.op(pegboard::ops::actor::log::read::Input {
    actor_ids: container_ids_clone.clone(),  // using actor_ids for containers!

6. Variable Naming Inconsistencies

Location: packages/core/api/actor/src/route/containers/logs.rs:161-164
Container logs code uses actor_id field names when processing container data:

if !container_id_to_index.contains_key(&entry.actor_id) {
    container_id_to_index.insert(entry.actor_id.clone(), ...);
    unique_container_ids.push(entry.actor_id.to_string());  

7. API Response Structure Differences

Inconsistency: Container logs include foreigns field but actor v1 logs don't:

• V1 actors logs: Has actor_indices but NO foreigns field (sdks/api/fern/definition/actors/v1/logs.yml:52-55)
• V2 actors logs: Has BOTH foreigns AND actor_indices fields (sdks/api/fern/definition/actors/logs.yml:51-56)
• Container logs: Has BOTH foreigns AND container_indices fields (sdks/api/fern/definition/containers/logs.yml:51-56)

---

⚡ Performance Concerns

8. Massive Code Duplication

Near-identical implementations across:

• Actor logs (v1 vs v2): ~240 lines duplicated
• Container vs Actor APIs: ~95% code similarity
• Multiple route definitions with same logic

Impact: Increased binary size, maintenance burden, potential for divergent behavior.

9. Inefficient Route Evaluation

The dual mounting of OldRouter may cause unnecessary route evaluation overhead for every request.

10. Expensive Actor Validation (packages/core/api/actor/src/assert.rs:275-337)

The actor validation system makes up to 16 concurrent HTTP requests to edge APIs for each validation batch, which could be expensive under load.

---

🔒 Security Assessment

Strengths:

• Proper authentication required on most endpoints
• Input validation through typed parameters
• Use of JSON parsing with error handling
• Actor validation caching with proper key management

Critical Gaps:

• No rate limiting on log endpoints (most critical)
• Missing authentication on deprecated endpoint
• No input size limits on *_ids_json parameters

---

🧪 Test Coverage Concerns

Cannot assess as tests are not included in this PR. Critical areas needing coverage:

• Rate limiting behavior (especially missing coverage for log endpoints)
• Route conflict resolution with dual mounting
• API consistency between actor/container endpoints
• Backward compatibility with deprecated endpoints

---

📝 Specific Recommendations

High Priority (Security)

1. Add rate limiting to ALL log endpoints - Critical security gap
2. Fix deprecated endpoint authentication (line 582-586)
3. Add input size limits on JSON parameters

Medium Priority (Correctness)

4. Resolve route mounting conflicts (lines 331-339)
5. Fix semantic mismatch in container logs using actor operations
6. Standardize ID types across API versions
7. Fix variable naming in container logs implementation

Low Priority (Maintenance)

8. Reduce code duplication through shared modules
9. Document API versioning strategy and migration paths
10. Add monitoring for v1 vs v2 API usage patterns

---

🎯 Overall Assessment

Status: ❌ Major Issues Require Resolution

While this PR successfully implements API versioning with backward compatibility, it introduces critical security vulnerabilities through missing rate limiting on log endpoints. The implementation also suffers from significant code duplication and API inconsistencies that will impact maintainability.

Priority: Fix security issues before merge, address correctness issues in follow-up PRs.

---

[cloudflare-workers-and-pages]

Deploying rivet-studio with    Cloudflare Pages

Latest commit:	f4ea5e9
Status:	🚫  Build failed.

View logs

[cloudflare-workers-and-pages]

Deploying rivet-hub with    Cloudflare Pages

Latest commit:	f4ea5e9
Status:	🚫  Build failed.

View logs

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[claude]

Claude finished @MasterPtato's task —— View job

---

🔍 PR Review Complete

Todo List

• Analyze changed files and understand the scope of API changes
• Review core routing and API handler implementations
• Check security aspects (authentication, rate limiting, input validation)
• Examine code quality and architectural decisions
• Identify potential bugs and edge cases
• Assess performance implications
• Evaluate test coverage considerations
• Provide comprehensive feedback with specific file references

---

📋 Summary of Changes

This PR implements major API versioning by:

1. Creating v2 actors API (/v2/actors) - new public API with proper versioning
2. Preserving v1 actors API (/actors, /v1/actors) - deprecated, internal-only via route mounting
3. Adding v1 containers API (/v1/containers) - new runtime API mirroring actor functionality
4. Maintaining backward compatibility through strategic route mounting

Scale: 39,815 additions, 5,038 deletions across 100 files

---

🚨 Critical Security Issues

1. Missing Rate Limiting on ALL Log Endpoints

CRITICAL: All log endpoints across all API versions lack rate limiting:

• V2 actor logs (packages/core/api/actor/src/route/mod.rs:120-125)
• V1 container logs (packages/core/api/actor/src/route/mod.rs:209-213)
• OldRouter actor logs (packages/core/api/actor/src/route/mod.rs:415-420)
• Deprecated V1 logs (packages/core/api/actor/src/route/mod.rs:582-586)

Impact: Potential DoS attacks, resource exhaustion, uncontrolled access to potentially expensive log operations.

Recommendation: Add rate limiting consistent with metrics endpoints:

rate_limit: {
    buckets: [
        { count: 100, bucket: duration::minutes(1) },
    ],
},

2. Deprecated Endpoint Lacks Authentication

Location: packages/core/api/actor/src/route/mod.rs:582-586
The deprecated V1 logs endpoint is missing opt_auth: true.

---

🔍 Code Quality & Architecture Issues

3. Route Mounting Conflicts

Location: packages/core/api/actor/src/route/mod.rs:331-339 and packages/edge/api/actor/src/route/mod.rs:188-196

mounts: [
    {
        path: OldRouter,
    },
    {
        path: OldRouter,
        prefix: "/v1"
    },
],

The same OldRouter is mounted twice, potentially causing routing conflicts or unnecessary overhead.

4. API Inconsistency: ID Types

• V1 actors: Use uuid type (sdks/api/fern/definition/actors/v1/__package__.yml:21,67)
• V2 actors: Use commons.Id type (packages/core/api/actor/src/route/actors/logs.rs:51)
• V1 containers: Use commons.Id type (sdks/api/fern/definition/containers/__package__.yml:20,66)

This creates inconsistent client experiences across API versions.

5. Semantic Mismatch: Container Logs Using Actor Operations

Location: packages/core/api/actor/src/route/containers/logs.rs:101,142
Container logs use pegboard::ops::actor::log::read::Input instead of container-specific operations:

ctx.op(pegboard::ops::actor::log::read::Input {
    actor_ids: container_ids_clone.clone(),  // using actor_ids for containers!

6. Variable Naming Inconsistencies

Location: packages/core/api/actor/src/route/containers/logs.rs:161-164
Container logs code uses actor_id field names when processing container data:

if !container_id_to_index.contains_key(&entry.actor_id) {
    container_id_to_index.insert(entry.actor_id.clone(), ...);
    unique_container_ids.push(entry.actor_id.to_string());  

7. API Response Structure Differences

Inconsistency: Container logs include foreigns field but actor v1 logs don't:

• V1 actors logs: Has actor_indices but NO foreigns field (sdks/api/fern/definition/actors/v1/logs.yml:52-55)
• V2 actors logs: Has BOTH foreigns AND actor_indices fields (sdks/api/fern/definition/actors/logs.yml:51-56)
• Container logs: Has BOTH foreigns AND container_indices fields (sdks/api/fern/definition/containers/logs.yml:51-56)

---

⚡ Performance Concerns

8. Massive Code Duplication

Near-identical implementations across:

• Actor logs (v1 vs v2): ~240 lines duplicated
• Container vs Actor APIs: ~95% code similarity
• Multiple route definitions with same logic

Impact: Increased binary size, maintenance burden, potential for divergent behavior.

9. Inefficient Route Evaluation

The dual mounting of OldRouter may cause unnecessary route evaluation overhead for every request.

10. Expensive Actor Validation (packages/core/api/actor/src/assert.rs:275-337)

The actor validation system makes up to 16 concurrent HTTP requests to edge APIs for each validation batch, which could be expensive under load.

---

🔒 Security Assessment

Strengths:

• Proper authentication required on most endpoints
• Input validation through typed parameters
• Use of JSON parsing with error handling
• Actor validation caching with proper key management

Critical Gaps:

• No rate limiting on log endpoints (most critical)
• Missing authentication on deprecated endpoint
• No input size limits on *_ids_json parameters

---

🧪 Test Coverage Concerns

Cannot assess as tests are not included in this PR. Critical areas needing coverage:

• Rate limiting behavior (especially missing coverage for log endpoints)
• Route conflict resolution with dual mounting
• API consistency between actor/container endpoints
• Backward compatibility with deprecated endpoints

---

📝 Specific Recommendations

High Priority (Security)

1. Add rate limiting to ALL log endpoints - Critical security gap
2. Fix deprecated endpoint authentication (line 582-586)
3. Add input size limits on JSON parameters

Medium Priority (Correctness)

4. Resolve route mounting conflicts (lines 331-339)
5. Fix semantic mismatch in container logs using actor operations
6. Standardize ID types across API versions
7. Fix variable naming in container logs implementation

Low Priority (Maintenance)

8. Reduce code duplication through shared modules
9. Document API versioning strategy and migration paths
10. Add monitoring for v1 vs v2 API usage patterns

---

🎯 Overall Assessment

Status: ❌ Major Issues Require Resolution

While this PR successfully implements API versioning with backward compatibility, it introduces critical security vulnerabilities through missing rate limiting on log endpoints. The implementation also suffers from significant code duplication and API inconsistencies that will impact maintainability.

Priority: Fix security issues before merge, address correctness issues in follow-up PRs.

---

[claude]

Claude finished @MasterPtato's task —— View job

---

🔍 PR Review Complete

Todo List

• Analyze the scope and structure of API versioning changes
• Review core routing and API handler implementations
• Check security aspects (authentication, rate limiting, input validation)
• Examine code quality and architectural decisions
• Identify potential bugs and edge cases
• Assess performance implications
• Evaluate test coverage considerations
• Provide comprehensive feedback with specific file references

---

📋 Summary of Changes

This PR implements major API versioning by:

1. Creating v2 actors API (/v2/actors) - new public API with proper versioning
2. Preserving v1 actors API (/actors, /v1/actors) - deprecated, internal-only via route mounting
3. Adding v1 containers API (/v1/containers) - new runtime API mirroring actor functionality
4. Maintaining backward compatibility through strategic route mounting

Scale: 39,815 additions, 5,038 deletions across 100 files

---

🚨 Critical Issues

1. Duplicate Route Definitions (packages/core/api/actor/src/route/mod.rs)

Lines 380-390 and 404-414 contain identical route definitions:

"actors" / "usage": {
    GET: actors::usage(...),
},
"actors" / "query": {
    GET: actors::query(...),
},

Impact: Routing conflicts and unpredictable behavior
Priority: CRITICAL - Must fix before merge

2. Missing Rate Limiting (Security Issue)

Missing rate limiting on log endpoints:

• v2 Actor logs (lines 120-125): /v2/actors/logs
• v1 Container logs (lines 209-214): /v1/containers/logs
• OldRouter actor logs (lines 463-468): /actors/logs

Impact: Potential DoS attacks, resource exhaustion
Priority: HIGH - Security vulnerability

3. Semantic Mismatch in Container Implementation

Location: packages/core/api/actor/src/route/containers/logs.rs

• Lines 101, 141: Uses pegboard::ops::actor::log::read::Input with actor_ids field for containers
• Lines 161-163: Variable names use actor_id when processing container data
• Lines 193: Maps actor_id to container_indices

Impact: Confusing code, potential bugs, maintenance issues
Priority: MEDIUM - Correctness issue

---

🔒 Security Assessment

Strengths:

• Consistent Authentication: All endpoints properly require authentication
• Comprehensive Error Handling: Proper error types and validation
• Rate Limiting: Most endpoints have appropriate rate limiting
• Input Validation: Proper parameter validation and type checking

Issues:

• Missing Rate Limiting: Log endpoints are vulnerable to abuse
• No Input Size Limits: JSON parameters like *_ids_json lack size constraints
• Route Mounting Conflicts: Dual mounting of OldRouter may cause security issues

---

⚡ Performance Concerns

1. Massive Code Duplication

Near-identical implementations across:

• Actor vs Container: ~95% code similarity in core handlers
• v1 vs v2 APIs: Duplicate route definitions with identical logic
• Edge vs Core: Same business logic implemented differently

Impact: Increased binary size, maintenance burden, potential for divergent behavior

2. Database Query Inefficiencies

Location: actors/mod.rs lines 85-126, containers/mod.rs lines 85-124

let mut futures = filtered_datacenters
    .into_iter()
    .map(|dc| async {
        // Individual HTTP call per datacenter
        actors_api::actors_get(&config, ...)
    })
    .collect::<futures_util::stream::FuturesUnordered<_>>();

Impact: N+1 query problem, high latency across datacenters

3. Route Mounting Overhead

Location: packages/core/api/actor/src/route/mod.rs lines 331-340

mounts: [
    { path: OldRouter, },
    { path: OldRouter, prefix: "/v1" },
],

Impact: Unnecessary route evaluation overhead for every request

---

🔍 Code Quality Issues

1. API Inconsistencies

• ID Types: v1 actors use uuid, v2 actors use commons.Id, containers use commons.Id
• Parameter Naming: Actor logs use actor_ids_json, container logs use container_ids_json
• Response Structure: Container logs include foreigns field, actor v1 logs don't

2. Circular Dependencies

Location: packages/core/api/actor/src/route/containers/mod.rs line 15

use crate::route::actors::resolve_dc;

Impact: Tight coupling between actor and container modules

3. Inefficient Error Handling

Location: Multiple locations in actor/container handlers

• Collects all errors but only returns the last one
• Different error handling patterns for identical logic

---

🧪 Test Coverage Issues

Missing Test Coverage:

• No tests for new logging endpoints (both v1 containers and v2 actors)
• No tests for metrics endpoints added in this PR
• No error handling tests for new error cases (invalid container IDs, invalid metrics)
• No integration tests for the new API versioning strategy

Existing Test Coverage:

• Core API tests exist with comprehensive integration tests
• CI/E2E tests are well-implemented with full lifecycle testing
• System tests exist for basic CRUD operations

---

📝 Specific Recommendations

High Priority (Must Fix)

1. Remove duplicate route definitions in packages/core/api/actor/src/route/mod.rs:404-426
2. Add rate limiting to ALL log endpoints - Critical security issue
3. Fix container logs semantic mismatch - Use container-specific operations and variable names

Medium Priority (Should Fix)

4. Standardize ID types across all API versions
5. Add input size limits for JSON parameters
6. Resolve route mounting conflicts - Clarify mounting strategy
7. Extract common functionality to reduce code duplication

Low Priority (Nice to Have)

8. Add comprehensive test coverage for new endpoints
9. Optimize database queries - Batch datacenter operations
10. Document API migration strategy for developers
11. Add performance monitoring for API versioning usage

---

🎯 Overall Assessment

Status: ⚠️ Approved with Critical Issues

This PR successfully implements API versioning with backward compatibility, but contains critical issues that must be addressed:

1. Duplicate route definitions will cause routing failures
2. Missing rate limiting creates security vulnerabilities
3. Semantic mismatches will cause maintenance headaches

The architecture is sound and the approach is well-thought-out, but the implementation has several critical bugs that need immediate attention.

Recommendation: Fix the critical issues before merge, address medium priority issues in follow-up PRs.

---

[claude]

Claude finished @MasterPtato's task —— View job

---

🔍 PR Review Complete

Todo List

• Analyze scope and structure of API versioning changes
• Review core routing and API handler implementations
• Check security aspects (authentication, rate limiting, input validation)
• Examine code quality and architectural decisions
• Identify potential bugs and edge cases
• Assess performance implications
• Evaluate test coverage considerations
• Provide comprehensive feedback with specific file references

---

📋 Summary of Changes

This PR implements major API versioning by:

1. Creating v2 actors API (/v2/actors) - new public API with proper versioning
2. Preserving v1 actors API (/actors, /v1/actors) - deprecated, internal-only via route mounting
3. Adding v1 containers API (/v1/containers) - new runtime API mirroring actor functionality
4. Maintaining backward compatibility through strategic route mounting

Scale: 39,815 additions, 5,038 deletions across 100 files

---

🚨 Critical Issues

1. Duplicate Route Definitions (packages/core/api/actor/src/route/mod.rs)

Lines 380-390 and 404-414 contain identical route definitions:

"actors" / "usage": {
    GET: actors::usage(...),
},
"actors" / "query": {
    GET: actors::query(...),
},

Impact: Routing conflicts and unpredictable behavior
Priority: CRITICAL - Must fix before merge

2. Missing Rate Limiting (Security Issue)

Missing rate limiting on log endpoints:

• v2 Actor logs (lines 120-125): /v2/actors/logs
• v1 Container logs (lines 209-214): /v1/containers/logs
• OldRouter actor logs (lines 463-468): /actors/logs

Impact: Potential DoS attacks, resource exhaustion
Priority: HIGH - Security vulnerability

3. Semantic Mismatch in Container Implementation

Location: packages/core/api/actor/src/route/containers/logs.rs

• Lines 101, 141: Uses pegboard::ops::actor::log::read::Input with actor_ids field for containers
• Lines 161-163: Variable names use actor_id when processing container data
• Lines 193: Maps actor_id to container_indices

Impact: Confusing code, potential bugs, maintenance issues
Priority: MEDIUM - Correctness issue

---

🔒 Security Assessment

Strengths:

• Consistent Authentication: All endpoints properly require authentication
• Comprehensive Error Handling: Proper error types and validation
• Rate Limiting: Most endpoints have appropriate rate limiting
• Input Validation: Proper parameter validation and type checking

Issues:

• Missing Rate Limiting: Log endpoints are vulnerable to abuse
• No Input Size Limits: JSON parameters like *_ids_json lack size constraints
• Route Mounting Conflicts: Dual mounting of OldRouter may cause security issues

---

⚡ Performance Concerns

1. Massive Code Duplication

Near-identical implementations across:

• Actor vs Container: ~95% code similarity in core handlers
• v1 vs v2 APIs: Duplicate route definitions with identical logic
• Edge vs Core: Same business logic implemented differently

Impact: Increased binary size, maintenance burden, potential for divergent behavior

2. Database Query Inefficiencies

Location: actors/mod.rs lines 85-126, containers/mod.rs lines 85-124

let mut futures = filtered_datacenters
    .into_iter()
    .map(|dc| async {
        // Individual HTTP call per datacenter
        actors_api::actors_get(&config, ...)
    })
    .collect::<futures_util::stream::FuturesUnordered<_>>();

Impact: N+1 query problem, high latency across datacenters

3. Route Mounting Overhead

Location: packages/core/api/actor/src/route/mod.rs lines 331-340

mounts: [
    { path: OldRouter, },
    { path: OldRouter, prefix: "/v1" },
],

Impact: Unnecessary route evaluation overhead for every request

---

🔍 Code Quality Issues

1. API Inconsistencies

• ID Types: v1 actors use uuid, v2 actors use commons.Id, containers use commons.Id
• Parameter Naming: Actor logs use actor_ids_json, container logs use container_ids_json
• Response Structure: Container logs include foreigns field, actor v1 logs don't

2. Circular Dependencies

Location: packages/core/api/actor/src/route/containers/mod.rs line 15

use crate::route::actors::resolve_dc;

Impact: Tight coupling between actor and container modules

3. Inefficient Error Handling

Location: Multiple locations in actor/container handlers

• Collects all errors but only returns the last one
• Different error handling patterns for identical logic

---

🧪 Test Coverage Issues

Missing Test Coverage:

• No tests for new logging endpoints (both v1 containers and v2 actors)
• No tests for metrics endpoints added in this PR
• No error handling tests for new error cases (invalid container IDs, invalid metrics)
• No integration tests for the new API versioning strategy

Existing Test Coverage:

• Core API tests exist with comprehensive integration tests
• CI/E2E tests are well-implemented with full lifecycle testing
• System tests exist for basic CRUD operations

---

📝 Specific Recommendations

High Priority (Must Fix)

1. Remove duplicate route definitions in packages/core/api/actor/src/route/mod.rs:404-426
2. Add rate limiting to ALL log endpoints - Critical security issue
3. Fix container logs semantic mismatch - Use container-specific operations and variable names

Medium Priority (Should Fix)

4. Standardize ID types across all API versions
5. Add input size limits for JSON parameters
6. Resolve route mounting conflicts - Clarify mounting strategy
7. Extract common functionality to reduce code duplication

Low Priority (Nice to Have)

8. Add comprehensive test coverage for new endpoints
9. Optimize database queries - Batch datacenter operations
10. Document API migration strategy for developers
11. Add performance monitoring for API versioning usage

---

🎯 Overall Assessment

Status: ⚠️ Approved with Critical Issues

This PR successfully implements API versioning with backward compatibility, but contains critical issues that must be addressed:

1. Duplicate route definitions will cause routing failures
2. Missing rate limiting creates security vulnerabilities
3. Semantic mismatches will cause maintenance headaches

The architecture is sound and the approach is well-thought-out, but the implementation has several critical bugs that need immediate attention.

Recommendation: Fix the critical issues before merge, address medium priority issues in follow-up PRs.

---

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-site	Error			Aug 21, 2025 8:19pm

[claude]

Claude finished @MasterPtato's task —— View job

---

🔍 PR Review Complete

This PR implements major API versioning changes (39,219 additions, 4,944 deletions across 100 files) introducing v2 actor APIs, preserving v1 actor APIs, and adding v1 container APIs.

Todo List

• Analyze scope and structure of API versioning changes
• Review core routing and API handler implementations
• Check security aspects (authentication, rate limiting, input validation)
• Examine code quality and architectural decisions
• Identify potential bugs and edge cases
• Assess performance implications
• Evaluate test coverage considerations
• Provide comprehensive feedback with specific file references

---

🚨 Critical Issues

1. Duplicate Route Definitions (packages/core/api/actor/src/route/mod.rs:380-426)

CRITICAL BUG: Lines 380-390 and 404-414 contain identical route definitions for:

• "actors" / "usage"
• "actors" / "query"

This will cause routing conflicts and unpredictable behavior. Must fix before merge.

2. Missing Rate Limiting (Security Vulnerability)

HIGH SECURITY RISK: Log endpoints lack rate limiting across all versions:

• v2 Actor logs (lines 120-125): /v2/actors/logs
• v1 Container logs (lines 209-214): /v1/containers/logs
• OldRouter actor logs (lines 463-468): /actors/logs

Impact: Potential DoS attacks and resource exhaustion.

3. Semantic Mismatch in Container Implementation

Location: packages/core/api/actor/src/route/containers/logs.rs

• Lines 101, 141: Uses pegboard::ops::actor::log::read::Input with actor_ids field for containers
• Lines 161-163: Variables named actor_id when processing container data
• Line 193: Maps actor_id to container_indices

Impact: Confusing code, potential bugs, maintenance issues.

---

🔒 Security Assessment

Strengths:

• Consistent Authentication: All endpoints require proper authentication
• Input Validation: Good parameter validation and type checking
• Rate Limiting: Most endpoints have appropriate rate limiting

Critical Gaps:

• Missing Rate Limiting: Log endpoints vulnerable to abuse
• Route Mounting Conflicts: Dual mounting may cause security issues
• No Input Size Limits: JSON parameters lack size constraints

---

🔍 Code Quality Issues

4. API Inconsistencies

ID Types:

• v1 actors: uuid type (sdks/api/fern/definition/actors/v1/__package__.yml:21,67)
• v2 actors: commons.Id type (sdks/api/fern/definition/actors/__package__.yml:20,66)
• v1 containers: commons.Id type (sdks/api/fern/definition/containers/__package__.yml:20,66)

Response Structure Differences:

• v1 actors logs: Has actor_indices but NO foreigns field
• v2 actors logs: Has BOTH foreigns AND actor_indices fields
• Container logs: Has BOTH foreigns AND container_indices fields

5. Route Mounting Issues

Location: packages/core/api/actor/src/route/mod.rs:331-339

mounts: [
    { path: OldRouter, },
    { path: OldRouter, prefix: "/v1" },
],

Same OldRouter mounted twice, potentially causing conflicts.

---

⚡ Performance Concerns

6. Massive Code Duplication

Near-identical implementations across:

• Actor vs Container APIs: ~95% code similarity
• v1 vs v2 APIs: Duplicate route definitions
• Core vs Edge: Same business logic

Impact: Increased binary size, maintenance burden, potential for divergent behavior.

7. Inefficient Route Evaluation

The dual mounting creates unnecessary route evaluation overhead for every request.

---

🧪 Test Coverage

Missing Coverage Areas:

• New logging endpoints (v2 actors, v1 containers)
• Route conflict resolution
• API versioning strategy
• Rate limiting behavior
• Backward compatibility

---

📝 Specific Recommendations

High Priority (Must Fix)

1. Remove duplicate route definitions in packages/core/api/actor/src/route/mod.rs:404-426
2. Add rate limiting to ALL log endpoints - Critical security issue
3. Fix container logs semantic mismatch - Use container-specific operations

Medium Priority

4. Standardize ID types across all API versions
5. Add input size limits for JSON parameters
6. Resolve route mounting conflicts - Clarify mounting strategy
7. Fix API response structure inconsistencies

Low Priority

8. Reduce code duplication through shared modules
9. Add comprehensive test coverage
10. Document API migration strategy

---

🎯 Overall Assessment

Status: ❌ Critical Issues Must Be Resolved

This PR implements a well-architected API versioning strategy with proper backward compatibility, but contains critical bugs that will cause system failures:

1. Duplicate route definitions will cause routing failures
2. Missing rate limiting creates serious security vulnerabilities
3. Semantic mismatches will cause maintenance headaches

Recommendation: Fix critical issues before merge, address medium priority issues in follow-up PRs.

---

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.