Repository navigation
Fix skipped mutable side effect replay recovery - #3164
Quinn-With-Two-Ns wants to merge 1 commit 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.
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.
| if (getState() == State.RESULT_NOTIFIED_REPLAYING && useReplayRecovery.apply()) { | ||
| notifySkippedReplaying(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Later flag replay shifts skip timing
High Severity
When MUTABLE_SIDE_EFFECT_REPLAY_RECOVERY is first recorded on a later workflow task, replay leaves earlier skipped invocations pending, then handleWorkflowTaskStarted resumes them at that later task's workflow time. The live task that wrote the flag did not re-run those continuations, so replay can emit a different command set and fail.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 9e5c687. Configure here.


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
High Risk
Changes workflow replay and command scheduling for mutable side effects, where subtle timing mistakes cause nondeterministic replay; mitigated by a default-off SDK flag and extensive replay tests.
Overview
Fixes skipped
Workflow.mutableSideEffectinvocations that could leave replay placeholders pending (and emit invalid empty RecordMarker commands) after sticky cache eviction or worker restart, especially when a workflow task completes with no matching marker events.Introduces SDK flag
MUTABLE_SIDE_EFFECT_REPLAY_RECOVERY(disabled by default for rollout). When enabled, replaying invocations are tracked separately; after all command events for a task are applied, pending skipped calls are resolved via the existing cancel/cached-result path so callbacks run at that task’s workflow time—not at the next task start, which would changeWorkflow.currentTimeMillis()and break replay.Flag usage is registered at
mutableSideEffectcall time when opted in viainitialFlags; histories without the flag keep prior timing.workflowTaskStartedalso resolves pending placeholders on live tasks when the flag is active. Command iteration during task start is copied to avoid concurrent modification when recovery runs workflow code.Adds state-machine, integration replay, callback-timing, and nondeterminism regression tests plus captured history fixtures.
Reviewed by Cursor Bugbot for commit 9e5c687. Bugbot is set up for automated code reviews on this repo. Configure here.