Flaky parallel crash-recovery test needs explicit step synchronization

[yqin0512]

Description

recovers from crashes during parallel step execution is nondeterministic because it assumes the successful step-a branch has persisted its completion before the failing step-b branch rejects Promise.all. There is no synchronization enforcing that ordering.

Source:

openworkflow/packages/openworkflow/worker/worker.test.ts

Lines 380 to 429 in 1b819b1

	test("recovers from crashes during parallel step execution", async () => {
	const backend = await createTestBackend();
	const client = new OpenWorkflow({ backend });
	let attemptCount = 0;
	const workflow = client.defineWorkflow(
	{ name: "crash-recovery" },
	async ({ step }) => {
	attemptCount++;
	const [a, b] = await Promise.all([
	step.run({ name: "step-a" }, () => {
	if (attemptCount > 1) return "x"; // should not happen since "a" will be cached
	return "a";
	}),
	step.run({ name: "step-b" }, () => {
	if (attemptCount === 1) throw new Error("Simulated crash");
	return "b";
	}),
	]);
	return { a, b, attempts: attemptCount };
	},
	);
	const worker = client.newWorker();
	const handle = await workflow.run();
	// first attempt will fail
	await worker.tick();
	const failedAttempt = await waitForWorkflowRun(
	backend,
	handle.workflowRun.id,
	(run) => run.status === "pending",
	"workflow to be rescheduled after simulated crash",
	);
	expect(attemptCount).toBe(1);
	// wait for backoff
	await sleepUntilAfter(failedAttempt.availableAt);
	// second attempt should succeed
	await worker.tick();
	const result = await handle.result();
	expect(result).toEqual({ a: "a", b: "b", attempts: 2 });
	expect(attemptCount).toBe(2);
	});

Observed failure

AssertionError: expected { a: 'x', b: 'b', attempts: 2 } to deeply equal { a: 'a', b: 'b', attempts: 2 }

- Expected
+ Received

  {
-   "a": "a",
+   "a": "x",
    "attempts": 2,
    "b": "b",
  }

The test passes most runs but fails intermittently under concurrent/cloud execution.

Race sequence

1. step-a and step-b start concurrently in Promise.all.
2. step-b throws Simulated crash.
3. The workflow is rescheduled and releases worker ownership before step-a durably records its successful output.
4. On the second workflow attempt, step-a is absent from the completed-step cache.
5. Its callback runs again and deliberately returns "x" because attemptCount > 1.

The assertion expects "a", so correctness currently depends on promise/database scheduling.

Suggested fix

Add explicit synchronization so step-b does not throw until step-a has completed its durable step write. For example, expose a deferred signal resolved after the step-a promise completes, and await it in the step-b branch before throwing. Synchronizing only inside the step-a callback is insufficient because persistence happens after that callback returns.

Alternatively, poll the backend for a completed step-a attempt before allowing step-b to fail. This would preserve the intended assertion: completed parallel work is cached across workflow retry.

Expected behavior

The test should deterministically establish that step-a is completed before triggering the simulated crash, then verify that replay reads "a" from durable history rather than executing its callback again.