Repository navigation
Fix skipped mutable side effect replay recovery - #3164
Open
Quinn-With-Two-Ns wants to merge 2 commits into
Open
Quinn-With-Two-Ns wants to merge 2 commits into
Quinn-With-Two-Ns wants to merge 2 commits into
Conversation
Resolve pending mutable side effect invocations at the original workflow-task boundary without changing legacy replay timing. Gate recovery behind a first-release-disabled SDK flag and register opt-in use at the mutableSideEffect call site. Add cache-eviction, callback-time, and nondeterminism detection replay coverage for issue #3163.
Quinn-With-Two-Ns
marked this pull request as ready for review
October 28, 2026 17:11
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 9e5c687. Configure here.
Capture recovery eligibility when each replay invocation is created. A flag first recorded on a later workflow task must not retime earlier unflagged continuations. Remove the task-start recovery hook and add a late-opt-in regression that also verifies new flagged calls complete at their own task time. Leave skip-count matching and the default-off rollout unchanged.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

What changed?
MUTABLE_SIDE_EFFECT_REPLAY_RECOVERY, disabled by default for its first release. Register opt-in usage at themutableSideEffectcall site and honor recorded flags during replay.Skip-count calculation and marker matching are unchanged.
Why?
Refs #3163.
A skipped mutable side effect has no corresponding marker event. During full replay, its internal placeholder can remain pending when there is no later command event to trigger cleanup, and an empty
RecordMarkercommand can reach the server.Unconditionally notifying the pending invocation at the next task start changes the workflow time observed by its continuation and can break replay. The added timing regression covers that case; flagged cleanup instead runs at the completed task's boundary, before the next task advances time or delivers signals.
Breaking changes?
None to public APIs. The new SDK flag remains disabled by default (
Enabled: pending) for rollout compatibility; tests opt in explicitly. Unflagged histories retain their existing replay behavior.Server PR
None required.
Validation
./gradlew --offline spotlessApplyadf0fc8) on the test and real servers. Baseline replay passed first, followed by 40 patched replay cases comparing exact callback values, order, and workflow times with opt-in off/on. The bulk corpus and generator are intentionally not included in this PR.Note
Medium Risk
Changes workflow replay state-machine behavior in a determinism-sensitive path, though the new behavior is gated behind a default-off SDK flag with extensive replay coverage.
Overview
Fixes replay after cache eviction or worker restart when a
Workflow.mutableSideEffectcall is skipped (no marker recorded): pending replay placeholders no longer stick around and emit invalidRecordMarkercommands.The change adds opt-in SDK flag
MUTABLE_SIDE_EFFECT_REPLAY_RECOVERY(introduced in 1.41.0, disabled by default). When enabled, replay tracks eligible invocations and, at workflow task completion (after that task’s command events), cancels still-pending placeholders and delivers the cached value so callbacks run at the original task time—not at the next task start, which would shiftWorkflow.currentTimeMillis()and break determinism. Flag use is recorded at themutableSideEffectcall site, and eligibility is fixed per invocation so a flag recorded on a later task cannot retime an earlier skipped call.CHANGELOG, state-machine unit tests, workflow replay tests (including captured histories), and negative nondeterminism checks are included; marker matching and skip-count logic are unchanged.
Reviewed by Cursor Bugbot for commit dcac469. Bugbot is set up for automated code reviews on this repo. Configure here.