Skip to content

fix(rivetkit-core): allow dispatch during sleep grace#4935

Merged
NathanFlurry merged 1 commit intomainfrom
05-04-fix_rivetkit_sleep_grace_dispatch
May 5, 2026
Merged

fix(rivetkit-core): allow dispatch during sleep grace#4935
NathanFlurry merged 1 commit intomainfrom
05-04-fix_rivetkit_sleep_grace_dispatch

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

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

Copy link
Copy Markdown
Member Author

NathanFlurry commented May 4, 2026

This stack of pull requests is managed by Graphite. Learn more about stacking.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 4, 2026

Code Review: fix(rivetkit-core): allow dispatch during sleep grace

Overview

This PR changes dispatch_lifecycle_error() in ActorTask so that actions arriving during SleepGrace are accepted and executed rather than immediately rejected with ActorLifecycleError::Stopping. Dispatch is still rejected during SleepFinalize and DestroyGrace. Two changes are bundled:

  1. SleepGrace is moved from the rejection arm to the allow arm in the match.
  2. The sleep_requested() early-return guard is removed entirely.

Correctness Analysis

SleepGrace dispatch: The change is mechanically sound. The event loop already accepted dispatches during SleepGrace via accepting_dispatch() -- they were just immediately rejected in handle_dispatch. Now they proceed to execution. Critically, handle_dispatch for Action calls self.ctx.wait_until(reply_future), which increments sleep_internal_keep_awake_count. Since can_finalize_shutdown() returns false while any keep-awake is held, the actor cannot leave SleepGrace until the action completes. The hard grace deadline (on_sleep_grace_deadline) still fires if work runs long, so there is no infinite deferral risk.

Removal of sleep_requested() guard: Before this PR, if ctx.sleep() was called but the engine Sleep Stop callback had not arrived yet (lifecycle still Started but sleep_requested = true), new dispatches were rejected. Removing this guard means dispatches in that brief window are now also allowed. The PR title says "allow dispatch during sleep grace" but this is also a behavioural change in the Started state during that window. A PR description note explaining this intentional widening would help reviewers.


Specific Feedback

Missing PR description (blocking): The description template is completely unfilled. Please describe what behaviour was broken, in what scenario dispatch during grace matters (e.g. an engine-initiated sleep arriving mid-request), and what was tested.

Grace deadline with in-flight dispatch (test gap): The existing test dispatches an action during SleepGrace and waits for it to complete before releasing the keep-awake. There is no test for the scenario where the grace deadline fires while an in-flight dispatch is still running. That path exercises on_sleep_grace_deadline() aborting the run handle and calling mark_shutdown_deadline_reached(). It would confirm the reply channel gets a sensible error rather than hanging.

sleep_requested() removal deserves a test: The removal looks like a pure cleanup but subtly changes behavior for the Started + sleep_requested=true window. A test named dispatch_accepted_while_sleep_requested_in_started_state would make the intent explicit and prevent future accidental reintroduction.

actors_notify scaffolding additions: The additions in context.rs, schedule.rs, and task.rs look correct. If they come from an unrelated struct change on the same branch they probably belong in a separate commit.

Test naming: sleep_grace_keeps_dispatch_open_and_second_sleep_is_idempotent tests two distinct properties. The idempotency of a second Stop Sleep is unrelated to dispatch behaviour. Splitting would improve clarity, but this is minor.

vec![7, 7, 7] assertion: This is the established test actor ping return value (line 383 of task.rs). No issue.


Summary

The core logic is correct: wait_until gates can_finalize_shutdown so accepting dispatch during grace cannot prevent the actor from ever sleeping. The main asks are: (1) fill out the PR description, (2) add a test for the grace-deadline-fires-with-in-flight-action path, (3) optionally add a focused test or comment for the sleep_requested + Started window change.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 4, 2026

Preview packages published to npm

Install with:

npm install rivetkit@pr-4935

All packages published as 0.0.0-pr.4935.dfe0098 with tag pr-4935.

Engine binary is shipped via @rivetkit/engine-cli on linux-x64-musl, linux-arm64-musl, darwin-x64, and darwin-arm64. Windows users should use the release installer or set RIVET_ENGINE_BINARY.

Docker images:

docker pull rivetdev/engine:slim-dfe0098
docker pull rivetdev/engine:full-dfe0098
Individual packages
npm install rivetkit@pr-4935
npm install @rivetkit/react@pr-4935
npm install @rivetkit/rivetkit-napi@pr-4935
npm install @rivetkit/workflow-engine@pr-4935

@NathanFlurry NathanFlurry force-pushed the 05-04-fix_rivetkit_sleep_grace_dispatch branch from 5ff98a4 to e054f95 Compare May 4, 2026 09:10
@NathanFlurry NathanFlurry force-pushed the 05-04-fix_rivetkit_sleep_idempotent_request branch from ea54634 to ae09be0 Compare May 4, 2026 09:10
Base automatically changed from 05-04-fix_rivetkit_sleep_idempotent_request to main May 5, 2026 14:57
@NathanFlurry NathanFlurry merged commit e054f95 into main May 5, 2026
38 of 44 checks passed
@NathanFlurry NathanFlurry deleted the 05-04-fix_rivetkit_sleep_grace_dispatch branch May 5, 2026 14:58
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