Skip to content

fix(rivetkit): keep actor alive while workflow waits for queue messages#5052

Draft
abcxff wants to merge 1 commit into
05-09-fix_workflow-engine_only_commit_step_state_after_successfrom
05-13-fix_rivetkit_keep_actor_alive_while_workflow_waits_for_queue_messages
Draft

fix(rivetkit): keep actor alive while workflow waits for queue messages#5052
abcxff wants to merge 1 commit into
05-09-fix_workflow-engine_only_commit_step_state_after_successfrom
05-13-fix_rivetkit_keep_actor_alive_while_workflow_waits_for_queue_messages

Conversation

@abcxff
Copy link
Copy Markdown
Contributor

@abcxff abcxff commented May 13, 2026

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 13, 2026

🚅 Deployed to the rivet-pr-5052 environment in rivet-frontend

Service Status Web Updated (UTC)
mcp-hub ✅ Success (View Logs) Web May 13, 2026 at 2:58 am
frontend-cloud ❌ Build Failed (View Logs) Web May 13, 2026 at 2:58 am
website ❌ Build Failed (View Logs) Web May 13, 2026 at 2:57 am
ladle ❌ Build Failed (View Logs) Web May 13, 2026 at 2:57 am
frontend-inspector ❌ Build Failed (View Logs) Web May 13, 2026 at 2:57 am
kitchen-sink ❌ Build Failed (View Logs) Web May 13, 2026 at 2:57 am

@abcxff abcxff mentioned this pull request May 13, 2026
11 tasks
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 13, 2026

Code Review

Overview

This PR fixes a bug in ActorWorkflowDriver.waitForMessages where the async queue wait was not wrapped with internalKeepAwake. Without this wrapper, the actor lifecycle system could decide the actor is idle and trigger a sleep/shutdown while a workflow was actively waiting for queue messages to arrive.

Correctness

The fix is correct and necessary. queueManager.waitForNames can block indefinitely — it resolves only when a matching queue message arrives — which is exactly the kind of long-lived async operation that needs internalKeepAwake to signal to the runtime that the actor is still doing meaningful work.

This was also the only method in ActorWorkflowDriver (and the sibling driver class above it) missing this wrapper. Every other async operation (enqueue, receive, completeMessage, completeMessageById, setAlarm, state gets/sets, etc.) already uses it. The omission looks like an oversight during the original implementation.

Consistency

The change follows the exact pattern used throughout the file and is minimally scoped. No concerns here.

Test Coverage

No test is added for the specific regression scenario: an actor should not sleep while a workflow is blocked in waitForMessages. The existing actor-workflow.test.ts covers queue-message consumption but doesn't exercise the sleep/keepAwake interaction for this code path.

Consider adding a test to actor-sleep.test.ts (or a dedicated test file) that:

  1. Starts a workflow that calls queue.next() (which triggers waitForMessages internally).
  2. Verifies the actor does not sleep while the workflow is blocked waiting.
  3. Sends a message and verifies the workflow proceeds correctly.

This would serve as a regression guard for the exact bug being fixed.

Minor Notes

  • The PR description template was left blank. Worth filling in before marking ready for review.
  • This is a draft PR — the fix itself looks complete, so the main blocker before promoting is test coverage for the regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant