[Graphite MQ] Draft PR GROUP:spec_75e3ca (PRs 4147, 4149, 4153, 4160, 4161, 4162, 4163, 4165)

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

• #4147
• #4149
• #4153
• #4160
• #4161
• #4162
• #4163
• #4165

[railway-app]

This PR was not deployed automatically as @graphite-app[bot] does not have access to the Railway project.

In order to get automatic PR deploys, please add @graphite-app[bot] to your workspace on Railway.

[claude]

Pull Request Review

This is a Graphite merge queue PR combining 8 separate PRs. Overall, the changes are well-structured and follow good practices. Here are my findings:

✅ Strengths

1. Strong Error Handling (fix(rivetkit): remove top-level promise errors #4161): The addition of rejection handlers to promiseWithResolvers across the codebase is excellent. This prevents unhandled promise rejections that can crash applications or cause subtle bugs.

2. Test Coverage (fix: add missing workflow fucntionality for queues #4165): Good addition of test cases for workflow queue functionality, including the new completeById feature and workflow context access restrictions.

3. Consistent Patterns: The codebase maintains consistent error handling patterns using structured logging with logger().warn().

4. Breaking Change Documentation (fix: rivetkit build #4147): PR description clearly identifies this as a breaking change with proper testing notes.

🔍 Issues Found

Critical Issues

1. Missing Rejection Handler Signature Update

   • Location: rivetkit-typescript/packages/rivetkit/src/utils.ts
   • Issue: The promiseWithResolvers function signature now requires an onReject parameter, but the return type only shows resolve and promise, not reject. Some call sites use the reject function (e.g., actor-driver.ts:427).
   • Fix: Ensure the return type includes reject: (reason?: any) => void if it's still exposed, or update all call sites to only use the onReject callback.

2. Workflow Access Test Timing Issues

   • Location: rivetkit-typescript/packages/rivetkit/src/driver-test-suite/tests/actor-workflow.ts:49-68
   • Issue: The test polls up to 20 times with 50ms delays (1 second total) but workflowAccessActor has a 25ms idle sleep. This tight timing could cause flakiness.
   • Fix: Consider increasing the polling interval or reducing iterations, and add a comment explaining the timing relationship.

Code Quality Issues

1. Unused Error Handling Pattern (fix(rivetkit): remove top-level promise errors #4161)

   • Location: rivetkit-typescript/packages/rivetkit/src/drivers/engine/actor-driver.ts:438-480
   • Issue: The try-catch block catches innerError but still throws an error, which will reject the actorStartPromise. The manual rejection seems redundant since throwing will reject the promise anyway.
   • Recommendation: Remove the manual handler.actorStartPromise?.reject(error) call or add a comment explaining why both are needed.

2. Inconsistent Logging Capitalization

   • Location: rivetkit-typescript/packages/rivetkit/src/actor/instance/queue-manager.ts:264
   • Issue: Per CLAUDE.md guidelines, log messages should be lowercase unless mentioning code symbols. Messages like "unhandled queue message waiter rejection" follow this correctly.
   • Verification: Ensure all new log messages follow this pattern consistently.

3. Duplicate Code in Vercel Examples

   • Location: examples/sandbox-vercel/src/actors/workflow/workflow-fixtures.ts and examples/sandbox/src/actors/workflow/workflow-fixtures.ts
   • Issue: Identical changes made to both files. This is expected per CLAUDE.md but increases maintenance burden.
   • Recommendation: Consider adding a CI check to verify Vercel examples stay in sync with regular examples.

Documentation Issues

1. Incomplete Error Messages

   • Location: Multiple files with onReject handlers
   • Issue: All rejection handlers use generic messages like "unhandled X rejection". These don't provide enough context for debugging.
   • Recommendation: Include the actor ID, name, or other identifying information in the log messages where available.

2. Missing Type Information (fix: add missing workflow fucntionality for queues #4165)

   • Location: rivetkit-typescript/packages/rivetkit/src/workflow/context.ts:109
   • Issue: The listen method now returns Promise<WorkflowListenMessage<T>> instead of Promise<T>, which is a breaking change not documented in the PR description.
   • Fix: Add migration notes for users upgrading.

🎯 Recommendations

High Priority

1. Add Migration Guide: Since fix: rivetkit build #4147 and fix: add missing workflow fucntionality for queues #4165 include breaking changes, create a migration guide showing:

   • How to update createFileSystemOrMemoryDriver calls
   • How to handle the new listen return type with message.body and message.complete()

2. Review Promise Rejection Patterns: Audit all new onReject handlers to ensure they:

   • Log sufficient context for debugging
   • Don't create memory leaks by retaining references
   • Handle rejection reasons appropriately

Medium Priority

1. Sentry Environment Configuration (feat(sentry): use correct env #4149)

   • The PR correctly adds VITE_APP_SENTRY_ENV but doesn't document what values are expected or how they map to Railway environments.
   • Add comments in .env files explaining valid values.

2. Test Flakiness Prevention

   • Consider adding a test utility for waiting with better error messages when conditions aren't met within timeouts.
   • Current pattern: for (let i = 0; i < 20 && state.insideDbCount === 0; i++) is hard to debug when it fails.

3. Type Safety for Queue Completion

   • completeById accepts response?: unknown which provides no type safety for completion responses.
   • Consider adding generic type parameters to maintain type safety through the queue completion flow.

Low Priority

1. Example Vercel Generation: Add skipVercel documentation to CLAUDE.md explaining when and why to use it.

2. Workflow History Caching: actorWorkflowHistoryQueryOptions uses staleTime: Infinity which means the history will never refetch. Consider if this is the desired behavior.

📊 Security & Performance

Security

✅ No security concerns identified. The changes don't introduce new attack vectors.

Performance

• Positive: Better error handling prevents memory leaks from unhandled rejections
• Neutral: New rejection handlers add minimal overhead
• Consider: The workflow history query with infinite stale time could cause memory issues for long-running workflows

🧪 Test Coverage

Good test coverage for new features:

• ✅ Workflow queue listen functionality
• ✅ Workflow context access restrictions
• ✅ Queue message completion with wait

Missing tests:

• ❌ Error scenarios for rejection handlers
• ❌ Edge cases for completeById with invalid IDs

Summary

This PR makes important improvements to error handling and workflow functionality. The main concerns are:

1. Verify the promiseWithResolvers signature exports all necessary functions
2. Document breaking changes for library users
3. Review timing assumptions in tests to prevent flakiness
4. Enhance error messages with more context

The changes are well-architected and follow the project's conventions. With the issues above addressed, this should be safe to merge.

Recommendation: Request changes to address the critical issues, then approve after fixes.