[workflows] Add Workflow rollback support by vaishnav-mk · Pull Request #13983 · cloudflare/workers-sdk

vaishnav-mk · 2026-05-20T09:32:47Z

Add saga rollback support to the local Workflows engine used by Wrangler dev.

Workflow steps can now register rollback handlers through trailing step.do() options and the local engine runs those handlers when the workflow fails. Rollbacks execute through rollback scoped Context.do() calls so they reuse the existing retry, timeout, attempt logging and step bookkeeping paths

This also aligns local rollback behavior with the service implementation by:

running rollbacks in reverse step-start order
re-registering rollback handlers on cached step results
emitting rollback-specific step and attempt events
disposing duplicated rollback RPC stubs after success/failure cleanup

Tests
- Tests included/updated
- Automated tests not possible - manual testing has been completed as follows:
- Additional testing not necessary because:
Public documentation
- Cloudflare docs PR(s): [Workflows] Document saga rollbacks cloudflare-docs#30976
- Documentation not necessary because:

A picture of a cute animal (not mandatory, but encouraged) (his name's leo)

changeset-bot · 2026-05-20T09:32:52Z

🦋 Changeset detected

Latest commit: 3ffb097

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@cloudflare/workflows-shared	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2026-05-20T09:33:52Z

✅ All changesets look good

ask-bonk · 2026-05-20T09:35:36Z

Review posted successfully on PR #13983. I found two actionable issues:

Missing closure guard (context.ts:225) — The ...rest: unknown[] refactor lost the implicit type safety of the old named parameters. When the config path is taken, rest[1] is cast to a function without validation, which would produce a confusing crash if a caller passes a malformed arg list. I posted an inline suggestion with a guard.
readLogs/readDetailedLogs stream output handling — These methods only resolve streamOutput metadata for STEP_SUCCESS events, but rollback steps now emit ROLLBACK_STEP_SUCCESS through the same code path that can attach streamOutput. If a rollback step returns a ReadableStream, the raw metadata will leak into log output. These lines are outside the diff so I noted the fix in the review body.

github run

pkg-pr-new · 2026-05-20T09:59:14Z

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13983

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13983

miniflare

npm i https://pkg.pr.new/miniflare@13983

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13983

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13983

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13983

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13983

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13983

@cloudflare/workers-utils

npm i https://pkg.pr.new/@cloudflare/workers-utils@13983

wrangler

npm i https://pkg.pr.new/wrangler@13983

commit: 3ffb097

vaishnav-mk · 2026-05-20T10:02:31Z

/bonk

ask-bonk · 2026-05-20T10:04:47Z

Review posted successfully on PR #13983. I found two actionable issues:

Missing curly braces (rollback.ts:149) — The for loop cleanup of remaining rollback stubs after a failure is missing curly braces, which will fail the repo's curly: error ESLint rule.
Non-null assertion (context.ts:306) — this.#rollbackStep!.cacheKey uses a ! assertion which is forbidden by the repo's lint config. The fix is to narrow directly on this.#rollbackStep instead of going through the isRollback boolean.

I also noted that the fatal abort paths (DataCloneError, etc.) skip rollbacks for earlier steps since abort() kills the DO before executeRollbacks can run — this seems intentionally correct but worth a code comment.

Both issues from the previous review (closure guard, streamOutput handling in readLogs/readDetailedLogs) have been properly addressed.

github run

vaishnav-mk · 2026-05-20T10:31:56Z

/bonk

ask-bonk

I'm Bonk, and I've reviewed your PR after the latest lint-fix commit (6f338dd).

Previously flagged issues — status:

Closure guard (context.ts:226) — Fixed. The typeof closure !== "function" guard is in place.
readLogs/readDetailedLogs streamOutput handling (engine.ts:111-116) — Fixed. The isStepSuccessEvent() helper correctly covers both STEP_SUCCESS and ROLLBACK_STEP_SUCCESS.
for loop braces (rollback.ts:146-148) — Fixed. Braces added.
Non-null assertion on rollback cache key (context.ts:299-301) — Fixed. Now narrows via const rollbackStep = this.#rollbackStep.

Remaining issue:

Non-null assertions on array index (rollback.ts:126, rollback.ts:147) — entries[i]! uses non-null assertions which violate the repo's @typescript-eslint/no-non-null-assertion: error rule (.oxlintrc.jsonc:47). While the index is always in-bounds due to the loop condition, the linter doesn't know that. The fix is to use an intermediate variable or use at() with a guard. See inline comments.

Overall: The implementation is clean and well-structured. Rollback execution through Context.do is a smart reuse of existing retry/timeout/logging machinery. The LIFO ordering via SQL STEP_START events is correct, test coverage is thorough, and the changeset is well-written. One lint issue remains to address.

github run

ask-bonk · 2026-05-20T10:35:35Z

Review posted successfully on PR #13983. Here's a summary:

Previously flagged issues — all addressed:

Closure guard (context.ts:226) — fixed
readLogs/readDetailedLogs streamOutput handling (engine.ts:111-116) — fixed via isStepSuccessEvent() helper
for loop braces (rollback.ts:146-148) — fixed
Non-null assertion on rollback cache key (context.ts:299-301) — fixed via proper narrowing

One remaining issue found:

Non-null assertions on array index (rollback.ts:126, rollback.ts:147) — entries[i]! violates the repo's @typescript-eslint/no-non-null-assertion: error rule. Posted an inline suggestion to either use for...of with .slice() for the cleanup loop, or extract with an undefined guard.

github run

vaishnav-mk · 2026-05-20T11:58:06Z

/bonk

ask-bonk

I'm Bonk, and I've reviewed the latest state of the PR (d1ba1fb).

All previously flagged issues are resolved:

Closure guard (context.ts:226) — Fixed. typeof closure !== "function" guard in place.
readLogs/readDetailedLogs streamOutput handling (engine.ts:111-116) — Fixed. isStepSuccessEvent() helper covers both STEP_SUCCESS and ROLLBACK_STEP_SUCCESS.
for loop braces (rollback.ts:146-148) — Fixed.
Non-null assertion on rollback cache key (context.ts:299-301) — Fixed via const rollbackStep = this.#rollbackStep narrowing.
Non-null assertions on array index (rollback.ts:125, 146-148) — Fixed. Now uses for...of with entries.entries() and .slice() destructuring, eliminating all entries[i]! patterns.

No new issues found. The implementation is clean — rollback execution through Context.do is a smart reuse of retry/timeout/logging machinery, LIFO ordering via SQL STEP_START events is correct, and test coverage is thorough. LGTM.

ask-bonk · 2026-05-20T11:59:30Z

Review posted on PR #13983. All five issues raised across the previous three review rounds have been properly addressed in the latest commits. No new issues found — the PR looks good.

github run

vaishnav-mk · 2026-05-20T18:24:43Z

/bonk review

ask-bonk

I've reviewed the full PR. All five issues raised across the previous four review rounds have been properly addressed:

Closure guard (context.ts:226) — typeof closure !== "function" guard is in place.
readLogs/readDetailedLogs streamOutput handling (engine.ts:111-116) — isStepSuccessEvent() helper correctly covers both STEP_SUCCESS and ROLLBACK_STEP_SUCCESS.
for loop braces (rollback.ts:146-148) — Fixed.
Non-null assertion on rollback cache key (context.ts:299-301) — Fixed via const rollbackStep = this.#rollbackStep narrowing.
Non-null assertions on array index (rollback.ts:125, 146-148) — Fixed. Now uses for...of with entries.entries() and .slice() destructuring.

The implementation is well-structured. A few highlights:

Smart reuse of Context.do for rollback execution means rollbacks automatically inherit retry, timeout, and attempt-logging machinery without duplicating any of that logic.
LIFO ordering via SQL STEP_START events is correct and matches the service implementation. The fallback to registry insertion order for entries missing from SQL (edge case) is a good defensive measure.
Event model is clean — the new ROLLBACK_* events (17-25) are sequential, the isStepSuccessEvent helper keeps the readLogs/readDetailedLogs stream handling DRY, and rollback steps don't count against the step limit.
RPC stub lifecycle is handled correctly: dup() on registration, Symbol.dispose on cleanup, clearRollbackRegistry on success, and dispose of remaining entries on rollback chain halt.
Test coverage is thorough — LIFO ordering, parallel step ordering, rollbackConfig propagation, rollback context correctness, failed-step rollbacks, partial registration, chain-halt on failure, and no-rollback-on-success.

No new issues found. LGTM.

ask-bonk · 2026-05-20T18:26:14Z

Review posted and approved on PR #13983. All previously raised issues have been addressed, and no new issues were found. The implementation is solid — rollback execution through Context.do is a clean reuse of existing retry/timeout/logging machinery, LIFO ordering is correct, RPC stub lifecycle is properly managed, and test coverage is thorough.

github run

workers-devprod · 2026-05-20T18:27:25Z

Codeowners approval required for this PR:

✅ @cloudflare/workflows
✅ @cloudflare/wrangler

Show detailed file reviewers

devin-ai-integration

Devin Review found 1 potential issue.

View 6 additional findings in Devin Review.

Co-authored-by: Olga Silva <78314353+pombosilva@users.noreply.github.com>

vaishnav-mk · 2026-05-21T09:22:58Z

/bonk

ask-bonk

I've reviewed all the outstanding feedback against the current state of the branch. Everything has already been addressed:

@pombosilva's comments:

Changeset title (.changeset/saga-rollbacks.md:5) — Already updated to "Add rollback support for local Workflows development" (no "saga").
Remove PR reference (.changeset/saga-rollbacks.md:11) — Already removed; now just mentions the workflows_step_rollback compat flag without referencing cloudflare/workerd#6330.
Move parseRollbackOptions to rollback.ts (context.ts) — Already done. The function lives in rollback.ts:43-84 and is imported in context.ts:18.
Blank line before ROLLBACK_START (instance.ts:128) — Already present at line 127.

Devin review:

Clear rollbackRegistry in attemptResume (engine.ts) — Already handled at engine.ts:1019 with clearRollbackRegistry(this.rollbackRegistry).

No remaining unaddressed feedback found.

ask-bonk · 2026-05-21T09:26:15Z

I reviewed all outstanding feedback on PR #13983. All five items from @pombosilva and Devin's review have already been addressed in the current state of the branch:

Changeset title updated (no "saga")
PR reference removed from changeset (just mentions compat flag)
parseRollbackOptions already moved to rollback.ts
Blank line before ROLLBACK_START already present
rollbackRegistry cleared in attemptResume already implemented

Posted a review comment confirming this on the PR.

github run

petebacondarwin

The changes here are all local to the workflows-shared package, so delegating full review and approval to the workflows team (e.g. @pombosilva).
Looked through the code and don't see any logical issues, but have not validated whether the behaviour is expected from a product point of view.
Approving on behalf of workers-sdk in general.

workers-devprod

Codeowners reviews satisfied

Co-authored-by: Olga Silva <78314353+pombosilva@users.noreply.github.com>

workers-devprod added this to workers-sdk May 20, 2026

github-project-automation Bot moved this to Untriaged in workers-sdk May 20, 2026

This comment was marked as resolved.

Sign in to view

ask-bonk Bot reviewed May 20, 2026

View reviewed changes

This comment was marked as resolved.

Sign in to view

ask-bonk Bot approved these changes May 20, 2026

View reviewed changes

vaishnav-mk changed the title ~~[workflows-shared] Add Workflow rollback support~~ [workflows] Add Workflow rollback support May 20, 2026

vaishnav-mk marked this pull request as ready for review May 20, 2026 18:27

vaishnav-mk requested a review from workers-devprod as a code owner May 20, 2026 18:27

workers-devprod requested review from a team and edmundhung and removed request for a team May 20, 2026 18:27

devin-ai-integration Bot reviewed May 20, 2026

View reviewed changes

Comment thread packages/workflows-shared/src/engine.ts

pombosilva reviewed May 21, 2026

View reviewed changes

Comment thread .changeset/saga-rollbacks.md Outdated

pombosilva reviewed May 21, 2026

View reviewed changes

Comment thread packages/workflows-shared/src/instance.ts

vaishnav-mk force-pushed the vaish/saga-rollbacks branch from fa8b197 to 90e7632 Compare May 21, 2026 09:21

vaishnav-mk and others added 13 commits May 21, 2026 14:52

[workflows-shared] Add step rollback support

dbf0e04

[workflows-shared] Test step rollback support

d8deb9e

[workflows-shared] Roll back failed steps

959c9f0

[workflows-shared] Align rollback execution with service

34526e5

[workflows-shared] Simplify rollback registration

cd58c4d

[workflows-shared] Add rollback attempt events

0f91abf

[workflows-shared] Address rollback review comments

6b44ed1

[workflows-shared] Fix rollback lint issues

48c4943

[workflows-shared] Remove rollback non-null assertions

e794404

[workflows-shared] Await rollback test workflows

57afae9

[workflows-shared] Avoid abort path in rollback tests

95421c6

Update saga-rollbacks.md

399e36a

Co-authored-by: Olga Silva <78314353+pombosilva@users.noreply.github.com>

[workflows-shared] Move rollback parsing and clear registry on resume

5915efe

ask-bonk Bot reviewed May 21, 2026

View reviewed changes

[workflows-shared] Restore WorkflowFatalError import

3ffb097

vaishnav-mk force-pushed the vaish/saga-rollbacks branch from 90e7632 to 3ffb097 Compare May 21, 2026 09:29

petebacondarwin approved these changes May 21, 2026

View reviewed changes

pombosilva self-requested a review May 21, 2026 10:11

pombosilva approved these changes May 21, 2026

View reviewed changes

workers-devprod approved these changes May 21, 2026

View reviewed changes

github-project-automation Bot moved this from Untriaged to Approved in workers-sdk May 21, 2026

pombosilva merged commit 9803586 into main May 21, 2026
58 checks passed

pombosilva deleted the vaish/saga-rollbacks branch May 21, 2026 10:13

github-project-automation Bot moved this from Approved to Done in workers-sdk May 21, 2026

workers-devprod mentioned this pull request May 21, 2026

Version Packages #13969

Merged

penalosa pushed a commit that referenced this pull request May 28, 2026

[workflows] Add Workflow rollback support (#13983)

c131cf1

Co-authored-by: Olga Silva <78314353+pombosilva@users.noreply.github.com>

Uh oh!

Conversation

vaishnav-mk commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

ask-bonk Bot commented May 20, 2026

Uh oh!

pkg-pr-new Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vaishnav-mk commented May 20, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

ask-bonk Bot commented May 20, 2026

Uh oh!

vaishnav-mk commented May 20, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

ask-bonk Bot commented May 20, 2026

Uh oh!

vaishnav-mk commented May 20, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented May 20, 2026

Uh oh!

vaishnav-mk commented May 20, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented May 20, 2026

Uh oh!

workers-devprod commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vaishnav-mk commented May 21, 2026

Uh oh!

ask-bonk Bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk Bot commented May 21, 2026

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

workers-devprod left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vaishnav-mk commented May 20, 2026 •

edited

Loading

changeset-bot Bot commented May 20, 2026 •

edited

Loading

github-actions Bot commented May 20, 2026 •

edited

Loading

pkg-pr-new Bot commented May 20, 2026 •

edited

Loading

workers-devprod commented May 20, 2026 •

edited

Loading