B1 — a recorded side effect is a state, not a return value

[gplanchat]

First audit finding addressed. hasSideEffectForSlot(): bool joins WorkflowHistorySourceInterface.

The defect

findSideEffectForSlot() returns mixed and signalled "nothing recorded" with null. A closure that legitimately returns null was therefore indistinguishable from an empty slot:

$replayResult = $this->historySource->findSideEffectForSlot($slotIndex);
if (null !== $replayResult) { /* replay */ }
$result = $closure();                                    // ← re-run on every pass
$this->commandBuffer->recordSideEffect($this->uuid(), $result);   // ← one more event per pass

A non-deterministic side effect replayed, a value that may differ from one pass to the next, and a journal that grows without bound — which is precisely the guarantee sideEffect() exists to give.

What the test taught us, and the report did not say

The data set covers the five values that look like absence — null, false, 0, '', []. Only null failed: the comparison was a strict !==, so the other four already replayed correctly. The defect was narrower than reported.

The other four stay in the data set as a regression net: a fix written with ?? or isset() would reopen the hole for all five. That is also what UPGRADE.md tells backend authors — array_key_exists(), never isset().

A fifth case pins slot alignment. It already passed — re-emissions landed at the end of the stream, so the indexing of the other slots stayed correct — but that was luck, and luck should not be load-bearing.

The fix

Slot presence is no longer inferred from the value the slot holds. Two implementations, two shapes: a journal walk for EventStoreHistorySource, an array_key_exists() for TemporalExecutionHistory. These are the only two implementations of the port in the repository — verified.

Break

For anyone who implements the port, that is, anyone writing a backend. No calling application is affected: it gets the fix for free.

Rector cannot help here, and UPGRADE.md says so rather than glossing over it: the answer depends on how the adapter stores its slots, and making Rector guess one would produce an adapter that compiles and lies. The note gives both reference shapes.

Verification

	main	this branch
unit suite	1073 tests, 4 errors	1089 tests, 4 errors
PHPStan	2 errors	2 errors

The 4 errors and the 2 diagnostics are pre-existing and identical: illuminate/cache is not installed on this machine, CI installs it through its matrix. No regression, no new diagnostic.

RED first: the three null cases failed before the fix (Failed asserting that 2 is identical to 1, 3 is identical to 1, 3 is identical to 2).

---

Addendum — 2026-09-04, after cross-review

Two claims above did not survive verification, and the branch has changed accordingly.

• The original text justified the shape by an existing separation for timers, {@see isTimerSettled()}. That method does not exist on WorkflowHistorySourceInterface. The real precedent is findScheduledTimerId() for state and findTimerSlotResult() for value — and the three sibling methods for activities, child workflows and Nexus operations, which wrap their result in array{result: mixed, ...} for exactly this reason. The docblock now says that.
• The fix closed the hole for sideEffect() and left it open in hasRecordedWorkAhead(), the "am I replaying?" signal version() depends on. An execution whose remaining work was made only of side effects was seen as having reached the end of its history: it took the new branch mid-replay and wrote its version marker into a history written before the change point existed. That is now closed too, with a regression test.