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

UPGRADE.md

@@ -22,6 +22,78 @@ vendor/bin/rector process src
 Le set est **cumulatif** : le passer une fois rattrape toutes les versions franchies d'un coup. Il
 ne contient que ce que Rector sait faire sans deviner ; tout le reste est écrit à la main ci-dessous.
 
+## Non publié
+
+### `WorkflowHistorySourceInterface` gagne `hasSideEffectForSlot()`
+
+**Qui est concerné** : uniquement qui **implémente** `WorkflowHistorySourceInterface` — c'est-à-dire
+qui écrit un backend. Une application qui appelle `sideEffect()` n'a rien à changer ; elle gagne le
+correctif sans rien faire.
+
+**Ce qui était cassé.** `findSideEffectForSlot()` rend `mixed` et signalait « rien d'enregistré » par
+`null`. Une closure qui rend légitimement `null` était donc indistinguable d'un slot vide : elle
+était **ré-exécutée à chaque passe de rejeu**, et le journal grossissait d'un `SideEffectRecorded`
+par passe. C'est la garantie même que `sideEffect()` existe pour offrir. Les valeurs `false`, `0`,
+`''` et `[]` n'étaient pas touchées — la comparaison était un `!==` strict.
+
+**Ce qu'il faut écrire.** Une méthode qui répond *le slot existe-t-il*, sans regarder ce qu'il porte.
+Rector ne peut rien ici : la réponse dépend de la façon dont votre backend range ses slots, et lui
+en faire deviner une produirait un adaptateur qui compile et ment. Les deux implémentations livrées
+donnent les deux formes attendues.
+
+Sur un journal parcouru :
+
+```php
+public function hasSideEffectForSlot(int $slot): bool
+{
+    $index = 0;
+    foreach ($this->eventStore->readStream($this->executionId) as $event) {
+        if ($event instanceof SideEffectRecorded) {
+            if ($index === $slot) {
+                return true;
+            }
+            ++$index;
+        }
+    }
+
+    return false;
+}
+```
+
+Sur un tableau indexé par slot — et c'est `array_key_exists()`, jamais `isset()`, qui rouvrirait
+exactement le trou que ce correctif ferme :
+
+```php
+public function hasSideEffectForSlot(int $slot): bool
+{
+    return \array_key_exists($slot, $this->sideEffects);
+}
+```
+
+`findSideEffectForSlot()` ne change pas de signature et garde son comportement : elle rend la valeur,
+et rend `null` aussi bien pour un slot absent que pour un slot portant `null`. C'est désormais écrit
+dans son contrat, et c'est `hasSideEffectForSlot()` qui décide s'il faut exécuter la closure.
+
+
+### `version()` cesse de basculer une exécution en vol
+
+**Qui est concerné** : toute application qui appelle `version()`. Rien à écrire ; le comportement
+change, en mieux, et il faut savoir en quoi.
+
+`version()` décide de rendre l'ancien comportement quand l'exécution est encore en train de
+rejouer. Ce signal se déduisait des quatre types de slot qui savent dire leur présence — activité,
+minuteur, workflow enfant, opération Nexus — et laissait les effets de bord de côté, pour la raison
+même que le correctif ci-dessus vient de lever : leur présence ne se lisait pas sans lire leur
+valeur.
+
+Conséquence : une exécution dont le travail restant devant elle n'était fait que d'effets de bord
+était vue comme arrivée au bout de son historique. Elle prenait la branche **neuve** au milieu d'un
+rejeu et y écrivait son marqueur de version — dans une histoire écrite avant que le point de
+changement existe. `hasSideEffectForSlot()` étant désormais au port, ce cas rejoint les autres.
+
+Une exécution qui a déjà écrit un marqueur de version garde le sien : `versionForChangeId()` est
+consulté en premier, et rien de ce commit ne le touche.
+
 ## 0.1.0-alpha8
 
 ### Laravel refuse au démarrage un workflow dont les noms de paramètres divergent du contrat

src/Bridge/Temporal/Worker/TemporalExecutionHistory.php

@@ -562,6 +562,11 @@ public function findScheduledTimerId(int $slot): ?string
         return $this->scheduledTimerIds[$slot] ?? null;
     }
 
+    public function hasSideEffectForSlot(int $slot): bool
+    {
+        return \array_key_exists($slot, $this->sideEffects);
+    }
+
     public function findSideEffectForSlot(int $slot): mixed
     {
         return $this->sideEffects[$slot] ?? null;

src/Durable/ExecutionContext.php

@@ -239,17 +239,20 @@ public function version(string $changeId, int $minSupported, int $maxSupported):
      * Déduit, donc déterministe : deux replays de la même histoire répondent pareil, ce qui est la
      * seule propriété dont le versioning a besoin.
      *
-     * Les effets de bord ne sont pas consultés : `findSideEffectForSlot()` rend `mixed`, et une
-     * valeur enregistrée peut légitimement être `null` — on ne peut pas distinguer « rien ici » de
-     * « ici, la valeur null ». Un workflow dont le seul travail avant un point de changement est
-     * un effet de bord sera donc traité comme neuf. C'est le trou, il est étroit, et il est écrit.
+     * Les effets de bord comptent comme les autres depuis que le port sait dire leur présence
+     * sans passer par leur valeur. Ils ne le pouvaient pas tant que `findSideEffectForSlot()`
+     * rendait `mixed` : une valeur enregistrée peut légitimement être `null`, et « rien ici » ne
+     * s'y distinguait pas de « ici, la valeur null ». Un workflow dont le seul travail avant un
+     * point de changement était un effet de bord basculait alors sur la branche neuve, en plein
+     * rejeu — le trou est fermé avec celui de `sideEffect()`, dont il était la même cause.
      */
     private function hasRecordedWorkAhead(): bool
     {
         return null !== $this->historySource->findScheduledActivityId($this->activitySlotIndex)
             || null !== $this->historySource->findScheduledTimerId($this->timerSlotIndex)
             || null !== $this->historySource->findScheduledChildExecutionId($this->childWorkflowSlotIndex)
-            || null !== $this->historySource->findScheduledNexusOperation($this->nexusOperationSlotIndex);
+            || null !== $this->historySource->findScheduledNexusOperation($this->nexusOperationSlotIndex)
+            || $this->historySource->hasSideEffectForSlot($this->sideEffectSlotIndex);
     }
 
     /**
@@ -311,10 +314,12 @@ private function refuseDivergence(string $slotKind, int $slotIndex, ?string $rec
     public function sideEffect(\Closure $closure): Awaitable
     {
         $slotIndex = $this->sideEffectSlotIndex++;
-        $replayResult = $this->historySource->findSideEffectForSlot($slotIndex);
         $deferred = new \Gplanchat\Durable\Awaitable\Deferred();
-        if (null !== $replayResult) {
-            $deferred->resolve($replayResult);
+
+        // La présence du slot, jamais la valeur qu'il porte : une closure qui rend `null` a bel et
+        // bien été exécutée, et la relire est exactement ce que `sideEffect()` promet.
+        if ($this->historySource->hasSideEffectForSlot($slotIndex)) {
+            $deferred->resolve($this->historySource->findSideEffectForSlot($slotIndex));
 
             return $deferred->awaitable();
         }

src/Durable/Port/WorkflowHistorySourceInterface.php

@@ -81,7 +81,25 @@ public function findTimerSlotResult(int $slot): ?array;
     public function findScheduledTimerId(int $slot): ?string;
 
     /**
-     * Returns the recorded side effect result at slot N, or null if not yet recorded.
+     * Whether slot N holds a recorded side effect.
+     *
+     * Presence is a fact about the history; the recorded value is data. They must be asked
+     * separately, because a side effect legitimately records `null` — and inferring "not recorded"
+     * from a `null` result re-runs a non-deterministic closure on every replay and appends a
+     * `SideEffectRecorded` per pass, which is the one guarantee `sideEffect()` exists to give.
+     *
+     * The same separation already exists on this port for timers, where
+     * {@see findScheduledTimerId()} answers the state and {@see findTimerSlotResult()} the value,
+     * and for activities, child workflows and Nexus operations, whose three sibling methods wrap
+     * their result in an `array{result: mixed, ...}` for exactly this reason.
+     */
+    public function hasSideEffectForSlot(int $slot): bool;
+
+    /**
+     * Returns the recorded side effect result at slot N.
+     *
+     * Returns `null` both for a slot that recorded `null` and for a slot that recorded nothing;
+     * callers deciding whether to run a closure MUST ask {@see hasSideEffectForSlot()} first.
      */
     public function findSideEffectForSlot(int $slot): mixed;
 

src/Durable/Store/EventStoreHistorySource.php

@@ -220,6 +220,21 @@ public function findScheduledTimerId(int $slot): ?string
         return null;
     }
 
+    public function hasSideEffectForSlot(int $slot): bool
+    {
+        $index = 0;
+        foreach ($this->eventStore->readStream($this->executionId) as $event) {
+            if ($event instanceof SideEffectRecorded) {
+                if ($index === $slot) {
+                    return true;
+                }
+                ++$index;
+            }
+        }
+
+        return false;
+    }
+
     public function findSideEffectForSlot(int $slot): mixed
     {
         $index = 0;

tests/unit/Durable/SideEffectSlotPresenceTest.php

@@ -0,0 +1,167 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Tests\Unit\Durable;
+
+use Gplanchat\Durable\Event\SideEffectRecorded;
+use Gplanchat\Durable\InMemoryWorkflowRunner;
+use Gplanchat\Durable\RegistryActivityExecutor;
+use Gplanchat\Durable\Store\InMemoryEventStore;
+use Gplanchat\Durable\Transport\InMemoryActivityTransport;
+use Gplanchat\Durable\Versioning\ChangePoint;
+use Gplanchat\Durable\WorkflowEnvironment;
+use Gplanchat\Durable\WorkflowRegistry;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+
+/**
+ * Un effet de bord est enregistré ou il ne l'est pas. C'est un état, et un état ne se déduit pas
+ * d'une valeur de retour : la valeur enregistrée peut légitimement être `null`, `false` ou `0`.
+ *
+ * Ces cas fixent la garantie que `sideEffect()` existe pour offrir — la closure ne tourne qu'une
+ * fois, quoi qu'elle rende — et le corollaire qu'on oublie plus facilement : un journal qui ne
+ * grossit pas d'un événement à chaque passe de rejeu.
+ */
+final class SideEffectSlotPresenceTest extends TestCase
+{
+    /**
+     * @return iterable<string, array{0: mixed}>
+     */
+    public static function valeursQuiSeConfondentAvecLAbsence(): iterable
+    {
+        yield 'null' => [null];
+        yield 'false' => [false];
+        yield 'zéro' => [0];
+        yield 'chaîne vide' => [''];
+        yield 'liste vide' => [[]];
+    }
+
+    #[DataProvider('valeursQuiSeConfondentAvecLAbsence')]
+    public function testUneClosureQuiRendUneValeurFausseNeTourneQuUneFois(mixed $valeur): void
+    {
+        $store = new InMemoryEventStore();
+        $appels = 0;
+
+        $workflow = static function (WorkflowEnvironment $wf) use ($valeur, &$appels): mixed {
+            return $wf->sideEffect(static function () use ($valeur, &$appels): mixed {
+                ++$appels;
+
+                return $valeur;
+            });
+        };
+
+        self::executer($store, 'exec-1', $workflow);
+        self::assertSame(1, $appels, 'la première passe exécute la closure');
+
+        self::executer($store, 'exec-1', $workflow);
+        self::assertSame(1, $appels, 'la passe de rejeu doit relire le résultat, pas le recalculer');
+    }
+
+    #[DataProvider('valeursQuiSeConfondentAvecLAbsence')]
+    public function testLeJournalNeGrossitPasDUnEvenementParPasse(mixed $valeur): void
+    {
+        $store = new InMemoryEventStore();
+
+        $workflow = static fn(WorkflowEnvironment $wf): mixed => $wf->sideEffect(static fn(): mixed => $valeur);
+
+        self::executer($store, 'exec-1', $workflow);
+        self::executer($store, 'exec-1', $workflow);
+        self::executer($store, 'exec-1', $workflow);
+
+        self::assertSame(
+            1,
+            self::compterEffetsDeBord($store, 'exec-1'),
+            'trois passes sur une seule instruction sideEffect() doivent laisser un seul événement',
+        );
+    }
+
+    #[DataProvider('valeursQuiSeConfondentAvecLAbsence')]
+    public function testLaValeurRelueEstLaValeurEnregistree(mixed $valeur): void
+    {
+        $store = new InMemoryEventStore();
+
+        $workflow = static fn(WorkflowEnvironment $wf): mixed => $wf->sideEffect(static fn(): mixed => $valeur);
+
+        self::assertSame($valeur, self::executer($store, 'exec-1', $workflow));
+        self::assertSame($valeur, self::executer($store, 'exec-1', $workflow), 'le rejeu rend la même valeur');
+    }
+
+    /**
+     * Les slots restent alignés : un effet de bord « faux » ne doit pas décaler celui d'après.
+     */
+    public function testUnEffetDeBordFauxNeDecalePasLeSlotSuivant(): void
+    {
+        $store = new InMemoryEventStore();
+
+        $workflow = static fn(WorkflowEnvironment $wf): array => [
+            'premier' => $wf->sideEffect(static fn(): mixed => null),
+            'second' => $wf->sideEffect(static fn(): string => 'après'),
+        ];
+
+        self::assertSame(['premier' => null, 'second' => 'après'], self::executer($store, 'exec-1', $workflow));
+        self::assertSame(['premier' => null, 'second' => 'après'], self::executer($store, 'exec-1', $workflow));
+        self::assertSame(2, self::compterEffetsDeBord($store, 'exec-1'));
+    }
+
+    private static function executer(InMemoryEventStore $store, string $executionId, \Closure $workflow): mixed
+    {
+        $runner = new InMemoryWorkflowRunner(
+            $store,
+            new InMemoryActivityTransport(),
+            new RegistryActivityExecutor(),
+            0,
+            new WorkflowRegistry(),
+        );
+
+        return $runner->run($executionId, $workflow);
+    }
+
+    private static function compterEffetsDeBord(InMemoryEventStore $store, string $executionId): int
+    {
+        $total = 0;
+        foreach ($store->readStream($executionId) as $event) {
+            if ($event instanceof SideEffectRecorded) {
+                ++$total;
+            }
+        }
+
+        return $total;
+    }
+
+    /**
+     * L'appelant frère. `version()` demande « suis-je en train de rejouer ? » à
+     * `hasRecordedWorkAhead()`, qui interrogeait les quatre autres types de slot et pas les effets
+     * de bord, faute de pouvoir en lire la présence sans en lire la valeur.
+     *
+     * Une exécution dont le travail restant devant elle n'est fait que d'effets de bord était donc
+     * vue comme arrivée au bout de son historique. Elle prenait la branche neuve **en plein
+     * rejeu**, et écrivait son marqueur de version au milieu d'une histoire écrite avant que le
+     * point de changement existe — ce que `version()` est précisément là pour empêcher.
+     */
+    public function testUnTravailRestantFaitDEffetsDeBordRetientLAncienneVersion(): void
+    {
+        $store = new InMemoryEventStore();
+
+        // Le code d'avant : deux effets de bord, aucun point de changement.
+        $avant = static fn(WorkflowEnvironment $wf): array => [
+            'premier' => $wf->sideEffect(static fn(): mixed => null),
+            'second' => $wf->sideEffect(static fn(): string => 'après'),
+        ];
+        self::executer($store, 'exec-1', $avant);
+
+        // Le code d'après, sur la même exécution : un point de changement s'est glissé entre les
+        // deux effets de bord, et le second est encore devant.
+        $apres = static fn(WorkflowEnvironment $wf): array => [
+            'premier' => $wf->sideEffect(static fn(): mixed => null),
+            'version' => $wf->version('changement-1', 1, 3),
+            'second' => $wf->sideEffect(static fn(): string => 'après'),
+        ];
+
+        self::assertSame(
+            ChangePoint::DEFAULT_VERSION,
+            self::executer($store, 'exec-1', $apres)['version'],
+            'une exécution en vol garde l\'ancien comportement tant qu\'il lui reste du journal à rejouer',
+        );
+    }
+}