fix: upgrade actors by build name

[MasterPtato]

Changes

[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.

---

🪧 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.

[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(toolchain): fix compat with deploying using podman #2571 : 2 dependent PRs (#2710 , #2713 )
• fix(toolchain): dont upgrade for deploys by default #2743
• fix(workflows): fix signal publish/listen race condition #2742
• fix: upgrade actors by build name #2741 👈 (View in Graphite)
• fix: gracefully handle prom failure for pb topo #2731
• 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

---

Code Review Complete

I've analyzed all the changes in this PR. Here's my comprehensive review:

Review Tasks:

• Read and analyze all changed files
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Examine test coverage
• Provide comprehensive feedback

---

📋 Summary

This PR fixes actor upgrade functionality by changing how actors are upgraded to use build name instead of all build tags. The changes span across 4 files with primarily functional improvements and minor cleanup.

🔍 Changes Overview

1. Client Test (client.ts): Added commented-out actor upgrade test code for future testing
2. Actor Setup (setup.rs): Added 5-second delay before cleanup to allow vector log collection
3. Pegboard Service (list_for_env.rs): Fixed typo in comment ("ds tags" → "actor tags")
4. Toolchain (mod.rs): Modified actor upgrade to use build name instead of all build tags

✅ Positive Aspects

• Clear intent: The title "fix: upgrade actors by build name" clearly matches the primary change
• Minimal scope: Changes are focused and don't introduce unnecessary complexity
• Logging consideration: Adding delay for vector log collection shows good operational awareness
• Comment accuracy: Fixed misleading comment in pegboard service

🔧 Code Quality & Best Practices

Good:

• Rust code follows idiomatic patterns
• JSON construction uses appropriate macros (json!)
• Error handling is preserved
• Code structure remains clean

Suggestions:

• Consider adding a constant for the 5-second delay in setup.rs:730 for better maintainability:

  const LOG_COLLECTION_DELAY_SECS: u64 = 5;
  tokio::time::sleep(std::time::Duration::from_secs(LOG_COLLECTION_DELAY_SECS)).await;

🐛 Bug Analysis

Critical Fix: The main change in toolchain/mod.rs:180-182 addresses what appears to be a significant bug:

• Before: tags: Some(serde_json::to_value(&build_tags)?)
• After: tags: Some(json!({ build::tags::NAME: build_name }))

This change is correct - upgrading actors should filter by build name specifically, not all build tags, which could include version and other metadata that shouldn't be used for actor selection.

No new bugs identified.

⚡ Performance Considerations

• Positive: The 5-second delay in cleanup is reasonable and won't impact normal operations
• Neutral: Other changes have minimal performance impact
• Consideration: The actor upgrade filtering change may improve performance by being more selective

🔒 Security Concerns

No security issues identified. The changes:

• Don't expose sensitive information
• Don't modify authentication/authorization
• Don't introduce new attack vectors
• Use safe JSON construction methods

🧪 Test Coverage

Current state:

• Added commented-out test code in client.ts shows testing was considered
• No new test files added for the core functionality

Recommendations:

• Consider adding an integration test to verify actors are properly upgraded by build name
• The commented test code should be uncommented and properly integrated if it's meant for testing

📊 Overall Assessment

This is a solid bug fix that:

• ✅ Addresses a specific issue with actor upgrades
• ✅ Maintains code quality standards
• ✅ Includes operational improvements (log collection delay)
• ✅ Has minimal risk of introducing new issues

Risk Level: Low
Recommendation: Approve with optional minor improvements

---

[cloudflare-workers-and-pages]

Deploying rivet with    Cloudflare Pages

Latest commit:	5a3772a
Status:	✅  Deploy successful!
Preview URL:	https://d0448483.rivet.pages.dev
Branch Preview URL:	https://07-10-fix-upgrade-actors-by.rivet.pages.dev

View logs

[claude]

Claude finished @MasterPtato's task —— View job

---

Code Review Complete

I've analyzed all the changes in this PR. Here's my comprehensive review:

Review Tasks:

• Read and analyze all changed files
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Examine test coverage
• Provide comprehensive feedback

---

📋 Summary

This PR fixes actor upgrade functionality by changing how actors are upgraded to use build name instead of all build tags. The changes span across 4 files with primarily functional improvements and minor cleanup.

🔍 Changes Overview

1. Client Test (client.ts): Added commented-out actor upgrade test code for future testing
2. Actor Setup (setup.rs): Added 5-second delay before cleanup to allow vector log collection
3. Pegboard Service (list_for_env.rs): Fixed typo in comment ("ds tags" → "actor tags")
4. Toolchain (mod.rs): Modified actor upgrade to use build name instead of all build tags

✅ Positive Aspects

• Clear intent: The title "fix: upgrade actors by build name" clearly matches the primary change
• Minimal scope: Changes are focused and don't introduce unnecessary complexity
• Logging consideration: Adding delay for vector log collection shows good operational awareness
• Comment accuracy: Fixed misleading comment in pegboard service

🔧 Code Quality & Best Practices

Good:

• Rust code follows idiomatic patterns
• JSON construction uses appropriate macros (json!)
• Error handling is preserved
• Code structure remains clean

Suggestions:

• Consider adding a constant for the 5-second delay in setup.rs:730 for better maintainability:

  const LOG_COLLECTION_DELAY_SECS: u64 = 5;
  tokio::time::sleep(std::time::Duration::from_secs(LOG_COLLECTION_DELAY_SECS)).await;

🐛 Bug Analysis

Critical Fix: The main change in toolchain/mod.rs:180-182 addresses what appears to be a significant bug:

• Before: tags: Some(serde_json::to_value(&build_tags)?)
• After: tags: Some(json!({ build::tags::NAME: build_name }))

This change is correct - upgrading actors should filter by build name specifically, not all build tags, which could include version and other metadata that shouldn't be used for actor selection.

No new bugs identified.

⚡ Performance Considerations

• Positive: The 5-second delay in cleanup is reasonable and won't impact normal operations
• Neutral: Other changes have minimal performance impact
• Consideration: The actor upgrade filtering change may improve performance by being more selective

🔒 Security Concerns

No security issues identified. The changes:

• Don't expose sensitive information
• Don't modify authentication/authorization
• Don't introduce new attack vectors
• Use safe JSON construction methods

🧪 Test Coverage

Current state:

• Added commented-out test code in client.ts shows testing was considered
• No new test files added for the core functionality

Recommendations:

• Consider adding an integration test to verify actors are properly upgraded by build name
• The commented test code should be uncommented and properly integrated if it's meant for testing

📊 Overall Assessment

This is a solid bug fix that:

• ✅ Addresses a specific issue with actor upgrades
• ✅ Maintains code quality standards
• ✅ Includes operational improvements (log collection delay)
• ✅ Has minimal risk of introducing new issues

Risk Level: Low
Recommendation: Approve with optional minor improvements

---

[claude]

Claude finished @MasterPtato's task —— View job

---

Code Review Analysis

Review Tasks:

• Read repository guidelines (CLAUDE.md)
• Analyze changed files and understand the changes
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Examine test coverage
• Provide comprehensive feedback

---

📋 Summary

This PR fixes actor upgrade functionality by changing how actors are upgraded to use build name instead of all build tags. The changes span across 4 files with primarily functional improvements and minor cleanup.

🔍 Changes Overview

1. Client Test (client.ts): Added commented-out actor upgrade test code for future testing
2. Actor Setup (setup.rs): Added 5-second delay before cleanup to allow vector log collection
3. Pegboard Service (list_for_env.rs): Fixed typo in comment ("ds tags" → "actor tags")
4. Toolchain (mod.rs): Modified actor upgrade to use build name instead of all build tags

✅ Positive Aspects

• Clear intent: The title "fix: upgrade actors by build name" clearly matches the primary change
• Minimal scope: Changes are focused and don't introduce unnecessary complexity
• Logging consideration: Adding delay for vector log collection shows good operational awareness
• Comment accuracy: Fixed misleading comment in pegboard service
• Better logging: Added informative log message showing how many actors were upgraded

🔧 Code Quality & Best Practices

Good:

• Rust code follows idiomatic patterns
• JSON construction uses appropriate macros (json!)
• Error handling is preserved
• Code structure remains clean
• Clear variable naming and comments

Suggestions:

• Consider adding a constant for the 5-second delay in setup.rs:730 for better maintainability:

  const LOG_COLLECTION_DELAY_SECS: u64 = 5;
  tokio::time::sleep(std::time::Duration::from_secs(LOG_COLLECTION_DELAY_SECS)).await;

🐛 Bug Analysis

Critical Fix: The main change in toolchain/mod.rs:180-182 addresses what appears to be a significant bug:

• Before: tags: Some(serde_json::to_value(&build_tags)?)
• After: tags: Some(json!({ build::tags::NAME: build_name }))

This change is correct - upgrading actors should filter by build name specifically, not all build tags, which could include version and other metadata that shouldn't be used for actor selection.

No new bugs identified.

⚡ Performance Considerations

• Positive: The 5-second delay in cleanup is reasonable and won't impact normal operations
• Neutral: Other changes have minimal performance impact
• Consideration: The actor upgrade filtering change may improve performance by being more selective

🔒 Security Concerns

No security issues identified. The changes:

• Don't expose sensitive information
• Don't modify authentication/authorization
• Don't introduce new attack vectors
• Use safe JSON construction methods

🧪 Test Coverage

Current state:

• Added commented-out test code in client.ts shows testing was considered
• No new test files added for the core functionality

Recommendations:

• Consider adding an integration test to verify actors are properly upgraded by build name
• The commented test code should be uncommented and properly integrated if it's meant for testing

📊 Overall Assessment

This is a solid bug fix that:

• ✅ Addresses a specific issue with actor upgrades
• ✅ Maintains code quality standards
• ✅ Includes operational improvements (log collection delay)
• ✅ Has minimal risk of introducing new issues

Risk Level: Low
Recommendation: Approve with optional minor improvements

---

[cloudflare-workers-and-pages]

Deploying rivet-studio with    Cloudflare Pages

Latest commit:	5a3772a
Status:	🚫  Build failed.

View logs

[cloudflare-workers-and-pages]

Deploying rivet-hub with    Cloudflare Pages

Latest commit:	5a3772a
Status:	✅  Deploy successful!
Preview URL:	https://15c22cdd.rivet-hub-7jb.pages.dev
Branch Preview URL:	https://07-10-fix-upgrade-actors-by.rivet-hub-7jb.pages.dev

View logs

[claude]

Claude finished @MasterPtato's task —— View job

---

PR Code Review

Review Tasks:

• Read repository guidelines (CLAUDE.md)
• Analyze changed files and understand the changes
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Examine test coverage
• Provide comprehensive feedback

📋 Change Summary

This PR fixes actor upgrade functionality by changing how actors are upgraded to use build name instead of all build tags. The main changes are:

1. Test File (client.ts): Added commented-out actor upgrade test code
2. Actor Setup (setup.rs): Added 5-second delay before cleanup for vector log collection
3. Pegboard Service (list_for_env.rs): Minor comment clarification
4. Toolchain (mod.rs): Core fix - Modified actor upgrade to filter by build name only

✅ Code Quality & Best Practices

Positive Aspects:

• Clear intent: The title and changes clearly match the problem being solved
• Minimal scope: Changes are focused and don't introduce unnecessary complexity
• Idiomatic Rust: Code follows Rust best practices and conventions
• Good logging: Added informative log messages for actor upgrade counts
• Operational awareness: Added delay for vector log collection shows good production considerations

Minor Suggestions:

• Consider extracting the 5-second delay in setup.rs:730 to a named constant for maintainability:

  const LOG_COLLECTION_DELAY_SECS: u64 = 5;
  tokio::time::sleep(std::time::Duration::from_secs(LOG_COLLECTION_DELAY_SECS)).await;

🐛 Bug Analysis

Critical Fix Identified: The main change in mod.rs:180-182 addresses a significant bug:

Before (inferred from context):

tags: Some(serde_json::to_value(&build_tags)?)

After:

tags: Some(json!({
    build::tags::NAME: build_name,
}))

Why this is correct: The old code was filtering actors by ALL build tags (including version, current flag, and other metadata), which could lead to:

• Incorrect actor selection
• Actors not being upgraded when they should be
• Potential conflicts with version-specific metadata

The new code correctly filters actors by build name only, which is the appropriate criteria for actor upgrades.

No new bugs identified in this change.

⚡ Performance Considerations

• Positive: The 5-second delay in cleanup is reasonable and won't impact normal operations
• Improvement: Actor upgrade filtering is now more selective and efficient
• Neutral: Other changes have minimal performance impact
• Dependency updates: Cargo.lock changes suggest dependency updates but no performance concerns

🔒 Security Assessment

No security issues identified. The changes:

• Don't expose sensitive information
• Don't modify authentication/authorization logic
• Don't introduce new attack vectors
• Use safe JSON construction methods (json! macro)
• Maintain proper error handling

🧪 Test Coverage

Current State:

• Added commented-out test code in client.ts (lines 197-203) shows testing was considered
• No new test files added for the core functionality

Recommendations:

• Consider uncommented the test code if it's meant for integration testing
• Add a focused test to verify actors are properly upgraded by build name specifically
• Test the edge case where multiple actors have the same build name but different versions

📊 Overall Assessment

This is a solid bug fix that addresses a real issue in the actor upgrade system:

Strengths:

• ✅ Fixes a clear bug in actor filtering logic
• ✅ Maintains code quality and follows best practices
• ✅ Includes operational improvements (log collection delay)
• ✅ Low risk of introducing new issues
• ✅ Clear logging for debugging and monitoring

Risk Level: Low - The change is focused, well-understood, and addresses a specific issue

Recommendation: Approve - This is a clean, focused fix that solves the stated problem

---

Final Notes:

• The change from filtering by all build tags to filtering by build name only is architecturally correct
• The 5-second delay for log collection is a thoughtful operational improvement
• Consider adding specific tests for the upgrade functionality to prevent regression

---

[claude]

Claude finished @MasterPtato's task —— View job

---

PR Code Review

Review Tasks:

• Read repository guidelines (CLAUDE.md)
• Analyze changed files and understand the changes
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security concerns
• Examine test coverage
• Provide comprehensive feedback

---

📋 Change Summary

This PR fixes actor upgrade functionality by changing how actors are upgraded to use build name instead of all build tags. The main changes are:

1. Test File (client.ts): Added commented-out actor upgrade test code
2. Actor Setup (setup.rs): Added 5-second delay before cleanup for vector log collection
3. Pegboard Service (list_for_env.rs): Minor comment clarification
4. Toolchain (mod.rs): Core fix - Modified actor upgrade to filter by build name only

---

✅ Code Quality & Best Practices

Positive Aspects:

• Clear intent: The title and changes clearly match the problem being solved
• Minimal scope: Changes are focused and don't introduce unnecessary complexity
• Idiomatic Rust: Code follows Rust best practices and conventions
• Good logging: Added informative log messages for actor upgrade counts at /home/runner/work/rivet/rivet/packages/toolchain/toolchain/src/tasks/build_publish/mod.rs:191-195
• Operational awareness: Added delay for vector log collection shows good production considerations

Minor Suggestions:

• Consider extracting the 5-second delay in setup.rs:730 to a named constant for maintainability:

  const LOG_COLLECTION_DELAY_SECS: u64 = 5;
  tokio::time::sleep(std::time::Duration::from_secs(LOG_COLLECTION_DELAY_SECS)).await;

🐛 Bug Analysis

Critical Fix Identified: The main change in /home/runner/work/rivet/rivet/packages/toolchain/toolchain/src/tasks/build_publish/mod.rs:180-182 addresses a significant bug:

After:

tags: Some(json!({
    build::tags::NAME: build_name,
})),

Why this is correct: The old code was filtering actors by ALL build tags (including version, current flag, and other metadata), which could lead to:

• Incorrect actor selection
• Actors not being upgraded when they should be
• Potential conflicts with version-specific metadata

The new code correctly filters actors by build name only, which is the appropriate criteria for actor upgrades.

No new bugs identified in this change.

⚡ Performance Considerations

• Positive: The 5-second delay in cleanup is reasonable and won't impact normal operations
• Improvement: Actor upgrade filtering is now more selective and efficient
• Neutral: Other changes have minimal performance impact
• Dependency updates: Cargo.lock changes suggest dependency updates but no performance concerns

🔒 Security Assessment

No security issues identified. The changes:

• Don't expose sensitive information
• Don't modify authentication/authorization logic
• Don't introduce new attack vectors
• Use safe JSON construction methods (json! macro)
• Maintain proper error handling

🧪 Test Coverage

Current State:

• Added commented-out test code in client.ts (lines 197-203) shows testing was considered
• No new test files added for the core functionality

Recommendations:

• Consider uncommented the test code if it's meant for integration testing
• Add a focused test to verify actors are properly upgraded by build name specifically
• Test the edge case where multiple actors have the same build name but different versions

📊 Overall Assessment

This is a solid bug fix that addresses a real issue in the actor upgrade system:

Strengths:

• ✅ Fixes a clear bug in actor filtering logic
• ✅ Maintains code quality and follows best practices
• ✅ Includes operational improvements (log collection delay)
• ✅ Low risk of introducing new issues
• ✅ Clear logging for debugging and monitoring

Risk Level: Low - The change is focused, well-understood, and addresses a specific issue

Recommendation: Approve - This is a clean, focused fix that solves the stated problem

---

Final Notes:

• The change from filtering by all build tags to filtering by build name only is architecturally correct
• The 5-second delay for log collection is a thoughtful operational improvement
• Consider adding specific tests for the upgrade functionality to prevent regression

---

[graphite-app]

Merge activity

• Jul 12, 12:58 AM UTC: NathanFlurry added this pull request to the Graphite merge queue.
• Jul 12, 1:00 AM UTC: CI is running for this pull request on a draft pull request (#2745) due to your merge queue CI optimization settings.
• Jul 12, 1:01 AM UTC: Merged by the Graphite merge queue via draft PR: #2745.