Add .rollback() api for workflow step promises

[LuisDuarte1]

Originally this PR: #6330 but slop PRed because of google auth shenanigans

adds steppromise wrapper with .rollback() api for workflow saga rollbacks. step.do() and step.waitforevent() now return steppromise which lets users register a compensation function before the step rpc fires.

• sleep/sleepuntil are unchanged.
• includes type declarations and tests.

[ask-bonk]

I'm Bonk, and I've done a quick review of your PR.

This PR adds a .rollback() API to workflow step promises, gated behind the workflows_step_rollback compat flag. step.do() and step.waitForEvent() now return a StepPromise that supports registering a compensation function before the RPC fires.

Issues found (highest severity first):

1. [HIGH] The type declarations in rpc.d.ts and generated snapshots change WorkflowStep.do() and waitForEvent() return types from Promise<T> to StepPromise<T> unconditionally, but the runtime behavior is gated behind a compat flag. When the flag is off, the raw RPC stub is used and returns a plain Promise with no .rollback() method. This is a type-level backward compatibility break for users who haven't opted in.
2. [MEDIUM] RollbackContext.output type is inconsistent across snapshot files: NonNullable<T> | undefined in .d.ts files vs T | undefined in .ts files. These should match.
3. [LOW] StepPromise overrides then() but does not override catch(). While catch() delegates through then() per ES spec for native promises, subclasses that override then() should also override catch() explicitly to be safe against runtimes or tooling that resolve catch independently.
4. [LOW] Missing $compatDisableFlag on the new compat flag. Other flags that will eventually become defaults include a disable flag so workers can opt out after the enable date. If this is intentional (experimental, no default date yet), no action needed — but worth confirming.

---

This review was generated by an AI assistant and may contain errors. Please verify all suggestions before applying.

[ask-bonk]

[HIGH] Backward compatibility: The return type of WorkflowStep.do() is changed from Promise<T> to StepPromise<T> unconditionally, but at runtime this only holds when the workflows_step_rollback compat flag is enabled. Without the flag, the raw RPC stub returns a plain Promise with no .rollback() method.

Users on older compat dates (flag off) will see StepPromise<T> in their editor, call .rollback(), and get a runtime error.

Consider keeping the return type as Promise<T> here and only advertising StepPromise<T> when the flag is known to be enabled (or document that .rollback() requires the flag).

[ask-bonk]

[MEDIUM] Inconsistent type: this file defines output: T | undefined, but the corresponding .d.ts files (rpc.d.ts, experimental/index.d.ts, latest/index.d.ts) define output: NonNullable<T> | undefined. These should be consistent — either all NonNullable<T> | undefined or all T | undefined.

Suggested change

	output: T | undefined;
	output: NonNullable<T> | undefined;

[ask-bonk]

[MEDIUM] Same inconsistency as the experimental .ts snapshot — T | undefined here vs NonNullable<T> | undefined in the .d.ts files.

Suggested change

	output: T | undefined;
	output: NonNullable<T> | undefined;

[ask-bonk]

[LOW] StepPromise overrides then() but relies on the ES spec for catch() delegation. For defensive correctness, consider also overriding catch() explicitly:

Suggested change

	override then<TResult1 = unknown, TResult2 = never>(
	onFulfilled?: ((value: unknown) => TResult1 | PromiseLike<TResult1>) | null,
	onRejected?: ((reason: unknown) => TResult2 | PromiseLike<TResult2>) | null
	): Promise<TResult1 | TResult2> {
	return this.#getPromise().then(onFulfilled, onRejected);
	}
	override then<TResult1 = unknown, TResult2 = never>(
	onFulfilled?: ((value: unknown) => TResult1 | PromiseLike<TResult1>) | null,
	onRejected?: ((reason: unknown) => TResult2 | PromiseLike<TResult2>) | null
	): Promise<TResult1 | TResult2> {
	return this.#getPromise().then(onFulfilled, onRejected);
	}
	override catch<TResult = never>(
	onRejected?: ((reason: unknown) => TResult | PromiseLike<TResult>) | null
	): Promise<unknown | TResult> {
	return this.#getPromise().catch(onRejected);
	}

[ask-bonk]

[LOW] No $compatDisableFlag is defined. If this flag will eventually get an enable date and become default, a disable flag should be added now so workers can opt out. If this is intentional because it's experimental with no planned default date, consider adding a comment noting that.

[ask-bonk]

Review posted on PR #6525. Here's a summary of the findings:

1. [HIGH] The type declarations (rpc.d.ts, all generated snapshots) unconditionally change WorkflowStep.do() and waitForEvent() return types from Promise<T> to StepPromise<T>, but the runtime wrapping is gated behind the workflows_step_rollback compat flag. Users without the flag will see .rollback() in their types but get a runtime error when calling it.

2. [MEDIUM] RollbackContext.output has inconsistent types across snapshot files — NonNullable<T> | undefined in .d.ts files vs T | undefined in .ts files. Posted suggestions to fix.

3. [LOW] StepPromise overrides then() but not catch(). Suggested adding an explicit catch() override for defensive correctness.

4. [LOW] No $compatDisableFlag on the new compat flag — flagged for confirmation that this is intentional.

github run

[github-actions]

The generated output of @cloudflare/workers-types has been changed by this PR. If this is intentional, run just generate-types to update the snapshot. Alternatively, you can download the full generated types:

Full Type Diff

diff -r types/generated-snapshot/experimental/index.d.ts bazel-bin/types/definitions/experimental/index.d.ts
3940,3941d3939
<   readonly __stdoutp: ArrayBuffer;
<   readonly __stderrp: ArrayBuffer;
3947,3949c3945,3947
<   __stdinp?: ReadableStream | "pipe";
<   __stdoutp?: "pipe" | "ignore";
<   __stderrp?: "pipe" | "ignore" | "combined";
---
>   stdin?: ReadableStream | "pipe";
>   stdout?: "pipe" | "ignore";
>   stderr?: "pipe" | "ignore" | "combined";
3952,3954c3950,3952
<   get stdin(): WritableStream | undefined;
<   get stdout(): ReadableStream | undefined;
<   get stderr(): ReadableStream | undefined;
---
>   readonly stdin: WritableStream | null;
>   readonly stdout: ReadableStream | null;
>   readonly stderr: ReadableStream | null;
3959,3961d3956
<   readonly __stdinp: WritableStream | null;
<   readonly __stdoutp: ReadableStream | null;
<   readonly __stderrp: ReadableStream | null;
diff -r types/generated-snapshot/experimental/index.ts bazel-bin/types/definitions/experimental/index.ts
3946,3947d3945
<   readonly __stdoutp: ArrayBuffer;
<   readonly __stderrp: ArrayBuffer;
3953,3955c3951,3953
<   __stdinp?: ReadableStream | "pipe";
<   __stdoutp?: "pipe" | "ignore";
<   __stderrp?: "pipe" | "ignore" | "combined";
---
>   stdin?: ReadableStream | "pipe";
>   stdout?: "pipe" | "ignore";
>   stderr?: "pipe" | "ignore" | "combined";
3958,3960c3956,3958
<   get stdin(): WritableStream | undefined;
<   get stdout(): ReadableStream | undefined;
<   get stderr(): ReadableStream | undefined;
---
>   readonly stdin: WritableStream | null;
>   readonly stdout: ReadableStream | null;
>   readonly stderr: ReadableStream | null;
3965,3967d3962
<   readonly __stdinp: WritableStream | null;
<   readonly __stdoutp: ReadableStream | null;
<   readonly __stderrp: ReadableStream | null;