[Graphite MQ] Draft PR GROUP:spec_68731c (PRs 3580, 3588, 3622, 3628, 3629, 3630, 3633)

[graphite-app]

This draft PR was created by the Graphite merge queue.
Trunk will be fast forwarded to the HEAD of this PR when CI passes, and the original PRs will be closed.

The following PRs are included in this draft PR:

• #3580
• #3588
• #3622
• #3628
• #3629
• #3630
• #3633

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
rivetkit-serverless	Error			Dec 12, 2025 0:50am

3 Skipped Deployments

Project	Deployment	Preview	Comments	Updated (UTC)
rivet-cloud	Ignored			Dec 12, 2025 0:50am
rivet-inspector	Ignored	Preview		Dec 12, 2025 0:50am
rivet-site	Ignored	Preview		Dec 12, 2025 0:50am

[claude]

Code Review - Graphite MQ PR (3580, 3588, 3622, 3628, 3629, 3630, 3633)

This PR combines 7 separate changes. Overall the code quality is good, but there are several areas that need attention before merging.

---

🐛 Critical Issues

1. Potential Data Race in revive_workflows_inner (engine/packages/gasoline/src/db/kv/debug.rs:1241-1389)

The function uses parallelization with FuturesUnordered but doesn't prevent overlapping ID ranges when parallelization is greater than 1. The boundary calculations may allow adjacent chunks to process the same workflow ID. The last chunk should go to u128::MAX to avoid overlap.

2. Unbounded String Allocation (engine/packages/gasoline/src/db/kv/debug.rs:1343)

The code allocates a new lowercased string for every workflow error encountered. If error messages are large or there are many workflows, this could cause excessive memory usage. Consider normalizing error_like once at the beginning instead.

---

⚠️ Important Issues

3. Silent Continuation on Wake Failure (engine/packages/gasoline/src/db/kv/debug.rs:695-698)

When attempting to wake a completed workflow, the code logs a warning and continues. This could hide legitimate issues. The previous version used ensure! which would fail the entire operation. Decide if this should be a hard error or at least track/count these cases and report them to the caller.

4. Database Interface Breaking Change (engine/packages/gasoline/src/db/kv/mod.rs:422)

The Database::from_pools method signature changed to Database::new and now requires a config parameter. Verified updates in api-builder, bootstrap, gasoline, engine, and serverless-backfill. Make sure no other callers exist in the codebase.

5. Error Context Additions (engine/packages/epoxy/src/ops/propose.rs)

Good additions of .context() calls for better error messages, but ensure these don't mask the underlying error types needed for retry logic.

---

📊 Performance Considerations

6. Transaction Timeout Constants (engine/packages/gasoline/src/db/kv/mod.rs:51 & debug.rs:38)

Two different early transaction timeout constants: Duration::from_millis(2500) in mod.rs and Duration::from_secs(3) in debug.rs. Extract to a shared constant or document why they differ.

7. Moka Cache for In-Flight Requests (guard-core changes)

The switch to moka cache (PR #3629) is good for memory-bounded caching, but verify cache eviction policies are appropriate for in-flight request tracking, TTL values are set correctly, and no request can be lost due to premature eviction.

8. Batch Signal Processing (engine/packages/epoxy/src/workflows/purger.rs:25)

Good optimization to batch up to 1024 signals! Verify this doesn't cause excessive memory usage if each signal has many keys.

---

🔒 Security Concerns

9. Config Path Error Message (engine/packages/config/src/lib.rs:106-110)

The new error message reveals path structure which is generally fine, but ensure this can't leak sensitive path information in production logs.

---

✅ Good Practices

1. Activity Error Names: Adding activity names to error messages (ActivityFailure, ActivityTimeout) greatly improves debugging
2. Structured Logging: Consistent use of tracing fields
3. Workflow Revive Command: The new wf revive command is well-designed with dry-run support
4. Serverless Backfill: Clean oneshot service implementation
5. Context Additions: Comprehensive .context() additions for better error messages

---

📝 Code Style & Conventions

10. Unused Result Pattern (several locations)

Per Rust conventions, use Ok(()) or name the binding with underscore prefix if partially used.

11. Workspace Dependency Formatting (Cargo.toml)

Inconsistent formatting in workspace dependencies - should have spaces after commas per Rust conventions.

---

🧪 Test Coverage

Missing Tests For:

• revive_workflows command with parallelization
• Edge cases in workflow revival (boundary conditions)
• Serverless backfill when configs are deleted mid-flight
• New error contexts in epoxy operations

Recommendation: Add integration tests for the new wf revive command given its complexity.

---

📋 Summary

Severity	Count
Critical	2
Important	3
Performance	3
Security	1
Style	2

Recommendation: Fix critical issues 1 and 2 before merging. Address important issues if time permits.

---

🔍 Files Needing Attention

1. engine/packages/gasoline/src/db/kv/debug.rs - Critical fixes needed
2. engine/packages/gasoline/src/db/kv/mod.rs - Verify all callers updated
3. Cargo.toml - Fix formatting consistency
4. Add tests for workflow revival functionality

---

Overall Assessment: The changes improve error handling and add useful functionality, but the critical issues in revive_workflows_inner need to be addressed before merging to prevent potential data corruption or excessive resource usage.