chore(pegboard): replace computing image size manually with using tar bytes read

[NathanFlurry]

Changes

[NathanFlurry]

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
• 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 👈 (View in Graphite)
• 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.

[cloudflare-workers-and-pages]

Deploying rivet-studio with    Cloudflare Pages

Latest commit:	492c606
Status:	🚫  Build failed.

View logs

[cloudflare-workers-and-pages]

Deploying rivet-hub with    Cloudflare Pages

Latest commit:	492c606
Status:	🚫  Deploy failed.

View logs

[cloudflare-workers-and-pages]

Deploying rivet with    Cloudflare Pages

Latest commit:	492c606
Status:	🚫  Build failed.

View logs

[graphite-app]

There's a spacing issue in the syntax: None =>{ should be None => { to maintain consistent Rust formatting style.

Suggested change

	None =>{
	None =>{

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

[claude]

Claude finished @NathanFlurry's task —— View job

---

Pull Request Review

Reviewing PR #2625: "chore(pegboard): replace computing image size manually with using tar bytes read"

Review Tasks:

• Read and analyze the changed files to understand actual modifications
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Assess test coverage needs
• Provide comprehensive feedback

---

⚠️ Critical Issue: PR Title Mismatch

The PR title suggests changes related to "computing image size manually with using tar bytes read", but the actual changes are completely different:

Actual Changes Made:

1. packages/edge/infra/client/isolate-v8-runner/src/main.rs:236

   • Changed: tracing::debug!("received pong");
   • To: tracing::trace!("received pong");

2. packages/edge/infra/client/manager/src/ctx.rs:374

   • Changed: Message::Pong(_) => tracing::debug!("received pong"),
   • To: Message::Pong(_) => tracing::trace!("received pong"),

These are simple logging level adjustments, not image size computation changes.

Note: The tar bytes read functionality mentioned in the PR title already exists and is working correctly in image_download_handler.rs:417-438 with the parse_tar_total_bytes function.

---

Technical Review

✅ Code Quality & Best Practices

• Excellent: Changes follow Rust logging best practices by using appropriate log levels
• Consistent: Both files updated identically, maintaining consistency across the codebase
• Clean: Minimal, focused changes with clear intent

✅ Potential Bugs or Issues

• No bugs identified: Changes are safe and preserve existing functionality
• Low risk: Only affects logging output, not business logic

✅ Performance Considerations

• Positive impact: Reducing log level from debug to trace decreases log volume in debug builds
• Appropriate: WebSocket pong messages are frequent and typically non-essential for debugging
• Improved efficiency: Less logging overhead, especially during high WebSocket activity

✅ Security Concerns

• No security issues: Changes only affect logging verbosity levels
• No data exposure: No changes to what information is logged

✅ Test Coverage

• No additional tests needed: Logging changes don't require unit tests
• Existing tests unaffected: No functional changes that would impact existing test coverage

---

Recommendations

1. 🔴 Critical: Fix PR Title

   • Update title to accurately reflect changes: chore(pegboard): reduce pong message log level to trace
   • Or: fix(logging): reduce WebSocket pong log noise in debug builds

2. 📝 Add PR Description

   • Explain rationale (e.g., "Reduce log noise from frequent WebSocket pong messages")
   • If this is part of a larger change, clarify the relationship

3. 🔍 Review Comment Resolution

   • The Graphite AI review mentioned a spacing issue with None =>{ vs None => {
   • This pattern doesn't appear in the current changes, suggesting it may have been resolved or was a false positive

---

Summary

Approval Status: ✅ APPROVED with title correction needed

The code changes are technically sound and improve system performance by reducing unnecessary logging. However, the misleading PR title creates confusion and should be corrected to accurately represent the changes made.

Impact: Low risk, positive performance improvement through reduced log verbosity.

---

[claude]

Claude finished @NathanFlurry's task —— View job

---

Pull Request Review

Reviewing PR #2625: "chore(pegboard): replace computing image size manually with using tar bytes read"

Review Tasks:

• Read and analyze the changed files to understand actual modifications
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Assess test coverage needs
• Provide comprehensive feedback

---

⚠️ Critical Issue: PR Title Mismatch

The PR title suggests changes related to "computing image size manually with using tar bytes read", but the actual changes are completely different:

Actual Changes Made:

1. packages/edge/infra/client/isolate-v8-runner/src/main.rs:236

   • Changed: tracing::debug!("received pong");
   • To: tracing::trace!("received pong");

2. packages/edge/infra/client/manager/src/ctx.rs:374

   • Changed: Message::Pong(_) => tracing::debug!("received pong"),
   • To: Message::Pong(_) => tracing::trace!("received pong"),

These are simple logging level adjustments, not image size computation changes.

Note: The tar bytes read functionality mentioned in the PR title already exists and is working correctly in image_download_handler.rs:417-438 with the parse_tar_total_bytes function.

---

Technical Review

✅ Code Quality & Best Practices

• Excellent: Changes follow Rust logging best practices by using appropriate log levels
• Consistent: Both files updated identically, maintaining consistency across the codebase
• Clean: Minimal, focused changes with clear intent

✅ Potential Bugs or Issues

• No bugs identified: Changes are safe and preserve existing functionality
• Low risk: Only affects logging output, not business logic

✅ Performance Considerations

• Positive impact: Reducing log level from debug to trace decreases log volume in debug builds
• Appropriate: WebSocket pong messages are frequent and typically non-essential for debugging
• Improved efficiency: Less logging overhead, especially during high WebSocket activity

✅ Security Concerns

• No security issues: Changes only affect logging verbosity levels
• No data exposure: No changes to what information is logged

✅ Test Coverage

• No additional tests needed: Logging changes don't require unit tests
• Existing tests unaffected: No functional changes that would impact existing test coverage

---

Recommendations

1. 🔴 Critical: Fix PR Title

   • Update title to accurately reflect changes: chore(pegboard): reduce pong message log level to trace
   • Or: fix(logging): reduce WebSocket pong log noise in debug builds

2. 📝 Add PR Description

   • Explain rationale (e.g., "Reduce log noise from frequent WebSocket pong messages")
   • If this is part of a larger change, clarify the relationship

3. 🔍 Review Comment Resolution

   • The Graphite AI review mentioned a spacing issue with None =>{ vs None => {
   • This pattern doesn't appear in the current changes, suggesting it may have been resolved or was a false positive

---

Summary

Approval Status: ✅ APPROVED with title correction needed

The code changes are technically sound and improve system performance by reducing unnecessary logging. However, the misleading PR title creates confusion and should be corrected to accurately represent the changes made.

Impact: Low risk, positive performance improvement through reduced log verbosity.

---

[claude]

Claude finished @NathanFlurry's task —— View job

---

Claude is working on your task

---

Pull Request Review

Reviewing PR #2625: "chore(pegboard): replace computing image size manually with using tar bytes read"

Review Tasks:

• Read and analyze the changed files to understand actual modifications
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Assess test coverage needs
• Provide comprehensive feedback

---

⚠️ Critical Issue: PR Title Mismatch

The PR title suggests changes related to "computing image size manually with using tar bytes read", but the actual changes are completely different:

Actual Changes Made:

1. packages/edge/infra/client/isolate-v8-runner/src/main.rs:236

   • Changed: tracing::debug!("received pong");
   • To: tracing::trace!("received pong");

2. packages/edge/infra/client/manager/src/ctx.rs:374

   • Changed: Message::Pong(_) => tracing::debug!("received pong"),
   • To: Message::Pong(_) => tracing::trace!("received pong"),

These are simple logging level adjustments, not image size computation changes.

Note: The tar bytes read functionality mentioned in the PR title already exists and is working correctly in image_download_handler.rs:417-438 with the parse_tar_total_bytes function.

---

Technical Review

✅ Code Quality & Best Practices

• Excellent: Changes follow Rust logging best practices by using appropriate log levels
• Consistent: Both files updated identically, maintaining consistency across the codebase
• Clean: Minimal, focused changes with clear intent

✅ Potential Bugs or Issues

• No bugs identified: Changes are safe and preserve existing functionality
• Low risk: Only affects logging output, not business logic

✅ Performance Considerations

• Positive impact: Reducing log level from debug to trace decreases log volume in debug builds
• Appropriate: WebSocket pong messages are frequent and typically non-essential for debugging
• Improved efficiency: Less logging overhead, especially during high WebSocket activity

✅ Security Concerns

• No security issues: Changes only affect logging verbosity levels
• No data exposure: No changes to what information is logged

✅ Test Coverage

• No additional tests needed: Logging changes don't require unit tests
• Existing tests unaffected: No functional changes that would impact existing test coverage

---

Recommendations

1. 🔴 Critical: Fix PR Title

   • Update title to accurately reflect changes: chore(pegboard): reduce pong message log level to trace
   • Or: fix(logging): reduce WebSocket pong log noise in debug builds

2. 📝 Add PR Description

   • Explain rationale (e.g., "Reduce log noise from frequent WebSocket pong messages")
   • If this is part of a larger change, clarify the relationship

3. 🔍 Review Comment Resolution

   • The Graphite AI review mentioned a spacing issue with None =>{ vs None => {
   • This pattern doesn't appear in the current changes, suggesting it may have been resolved or was a false positive

---

Summary

Approval Status: ✅ APPROVED with title correction needed

The code changes are technically sound and improve system performance by reducing unnecessary logging. However, the misleading PR title creates confusion and should be corrected to accurately represent the changes made.

Impact: Low risk, positive performance improvement through reduced log verbosity.

---

[claude]

Claude finished @NathanFlurry's task —— View job

---

Pull Request Review

Reviewing PR #2625: "chore(pegboard): replace computing image size manually with using tar bytes read"

Review Tasks:

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

---

⚠️ Critical Issue: PR Title Mismatch

The PR title suggests changes related to "computing image size manually with using tar bytes read", but the actual changes are completely different:

Actual Changes Made:

1. packages/edge/infra/client/isolate-v8-runner/src/main.rs:236

   • Changed: tracing::debug!("received pong");
   • To: tracing::trace!("received pong");

2. packages/edge/infra/client/manager/src/ctx.rs:374

   • Changed: Message::Pong(_) => tracing::debug!("received pong"),
   • To: Message::Pong(_) => tracing::trace!("received pong"),

These are simple logging level adjustments, not image size computation changes.

Note: The tar bytes read functionality mentioned in the PR title already exists and is working correctly in image_download_handler.rs:417-438 with the parse_tar_total_bytes function.

---

Technical Review

✅ Code Quality & Best Practices

• Excellent: Changes follow Rust logging best practices by using appropriate log levels
• Consistent: Both files updated identically, maintaining consistency across the codebase
• Clean: Minimal, focused changes with clear intent

✅ Potential Bugs or Issues

• No bugs identified: Changes are safe and preserve existing functionality
• Low risk: Only affects logging output, not business logic

✅ Performance Considerations

• Positive impact: Reducing log level from debug to trace decreases log volume in debug builds
• Appropriate: WebSocket pong messages are frequent and typically non-essential for debugging
• Improved efficiency: Less logging overhead, especially during high WebSocket activity

✅ Security Concerns

• No security issues: Changes only affect logging verbosity levels
• No data exposure: No changes to what information is logged

✅ Test Coverage

• No additional tests needed: Logging changes don't require unit tests
• Existing tests unaffected: No functional changes that would impact existing test coverage

---

Recommendations

1. 🔴 Critical: Fix PR Title

   • Update title to accurately reflect changes: chore(pegboard): reduce pong message log level to trace
   • Or: fix(logging): reduce WebSocket pong log noise in debug builds

2. 📝 Add PR Description

   • Explain rationale (e.g., "Reduce log noise from frequent WebSocket pong messages")
   • If this is part of a larger change, clarify the relationship

3. 🔍 Review Comment Resolution

   • The Graphite AI review mentioned a spacing issue with None =>{ vs None => {
   • This pattern doesn't appear in the current changes, suggesting it may have been resolved or was a false positive

---

Summary

Approval Status: ✅ APPROVED with title correction needed

The code changes are technically sound and improve system performance by reducing unnecessary logging. However, the misleading PR title creates confusion and should be corrected to accurately represent the changes made.

Impact: Low risk, positive performance improvement through reduced log verbosity.

---

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-site	Ready	Preview	Comment	Aug 21, 2025 8:32pm

[claude]

Claude finished @NathanFlurry's task —— View job

---

Pull Request Review

Reviewing PR #2625: "chore(pegboard): replace computing image size manually with using tar bytes read"

Review Tasks:

• Read and analyze the changed files to understand actual modifications
• Review code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Evaluate security concerns
• Assess test coverage needs
• Provide comprehensive feedback

---

⚠️ Critical Issue: PR Title Mismatch

The PR title suggests changes related to "computing image size manually with using tar bytes read", but the actual changes are completely different:

Actual Changes Made:

1. packages/edge/infra/client/isolate-v8-runner/src/main.rs:236

   • Changed: tracing::debug!("received pong");
   • To: tracing::trace!("received pong");

2. packages/edge/infra/client/manager/src/ctx.rs:374

   • Changed: Message::Pong(_) => tracing::debug!("received pong"),
   • To: Message::Pong(_) => tracing::trace!("received pong"),

These are simple logging level adjustments, not image size computation changes.

Note: The tar bytes read functionality mentioned in the PR title already exists and is working correctly in image_download_handler.rs:417-438 with the parse_tar_total_bytes function.

---

Technical Review

✅ Code Quality & Best Practices

• Excellent: Changes follow Rust logging best practices by using appropriate log levels
• Consistent: Both files updated identically, maintaining consistency across the codebase
• Clean: Minimal, focused changes with clear intent

✅ Potential Bugs or Issues

• No bugs identified: Changes are safe and preserve existing functionality
• Low risk: Only affects logging output, not business logic

✅ Performance Considerations

• Positive impact: Reducing log level from debug to trace decreases log volume in debug builds
• Appropriate: WebSocket pong messages are frequent and typically non-essential for debugging
• Improved efficiency: Less logging overhead, especially during high WebSocket activity

✅ Security Concerns

• No security issues: Changes only affect logging verbosity levels
• No data exposure: No changes to what information is logged

✅ Test Coverage

• No additional tests needed: Logging changes don't require unit tests
• Existing tests unaffected: No functional changes that would impact existing test coverage

---

Recommendations

1. 🔴 Critical: Fix PR Title

   • Update title to accurately reflect changes: chore(pegboard): reduce pong message log level to trace
   • Or: fix(logging): reduce WebSocket pong log noise in debug builds

2. 📝 Add PR Description

   • Explain rationale (e.g., "Reduce log noise from frequent WebSocket pong messages")
   • If this is part of a larger change, clarify the relationship

3. 🔍 Review Comment Resolution

   • The Graphite AI review mentioned a spacing issue with None =>{ vs None => {
   • This pattern doesn't appear in the current changes, suggesting it may have been resolved or was a false positive

---

Summary

Approval Status: ✅ APPROVED with title correction needed

The code changes are technically sound and improve system performance by reducing unnecessary logging. However, the misleading PR title creates confusion and should be corrected to accurately represent the changes made.

Impact: Low risk, positive performance improvement through reduced log verbosity.

---

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