From 0b878ff561cadafdbb3c3c6665bcb44c280f3322 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Planchat?= Date: Wed, 2 Sep 2026 20:20:56 +0200 Subject: [PATCH 1/2] =?UTF-8?q?fix(stubs):=20les=20arguments=20nomm=C3=A9s?= =?UTF-8?q?=20ne=20se=20perdent=20plus=20en=20silence?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Les trois stubs `__call` — activité, opération Nexus, workflow enfant — transforment les arguments reçus en charge nommée, parce que c'est nommé que ça voyage dans le journal. Tous les trois le faisaient à l'identique : $payload[$param->getName()] = $arguments[$i] ?? (… défaut …); **PHP passe les arguments nommés à `__call` dans un tableau à clés de chaînes.** Appariés par indice, ils ne répondent à aucun `$arguments[$i]` — et *tous* les paramètres retombent sur leur valeur par défaut. Sans exception, sans avertissement, sans trace. C'est ce qui l'a fait sortir : un workflow enfant démarré par `->run(prompt: $mission, model: 'ministral-3b-latest', maxTurns: 1)` démarrait avec `prompt: null`, `maxTurns: 20` et le modèle par défaut, puis attendait un message qui ne viendrait jamais. Diagnostiqué en lisant la charge de `WORKFLOW_EXECUTION_STARTED` dans l'historique Temporal ; rien côté PHP ne l'aurait dit. Trois comportements, dont deux étaient silencieux : | | avant | après | |---|---|---| | argument nommé | perdu, valeur par défaut | apparié à son paramètre | | `null` explicite | valeur par défaut — l'inverse de ce qui est demandé | `null` | | nom inconnu | avalé, indiscernable d'un défaut voulu | `BadMethodCallException` | Le deuxième vient de `??`, qui confond « absent » et « null » ; d'où `array_key_exists`. Le troisième s'aligne sur ce que PHP fait d'un appel ordinaire (`Unknown named parameter`). Un point unique, `StubArguments::toPayload()`, appelé par les trois : ils faisaient déjà la même chose avec la même erreur, et un correctif partiel les aurait fait diverger. Aucun appel du dépôt n'utilisait d'arguments nommés sur un stub — balayage fait, ce qui explique la longévité du défaut. Les appels positionnels sont inchangés, à une exception près : un `null` passé explicitement vaut désormais `null` et non le défaut. C'est le correctif, pas une régression. 563 tests d'activité, Nexus, workflow enfant et stubs au vert. Co-Authored-By: Claude Opus 5 (1M context) --- src/Durable/Activity/ActivityStub.php | 11 +-- src/Durable/Nexus/NexusStub.php | 9 +- src/Durable/Stub/StubArguments.php | 87 +++++++++++++++++++ src/Durable/Workflow/ChildWorkflowStub.php | 10 +-- tests/unit/Durable/Stub/StubArgumentsTest.php | 84 ++++++++++++++++++ 5 files changed, 177 insertions(+), 24 deletions(-) create mode 100644 src/Durable/Stub/StubArguments.php create mode 100644 tests/unit/Durable/Stub/StubArgumentsTest.php diff --git a/src/Durable/Activity/ActivityStub.php b/src/Durable/Activity/ActivityStub.php index c1d2ce4c..25362b9b 100644 --- a/src/Durable/Activity/ActivityStub.php +++ b/src/Durable/Activity/ActivityStub.php @@ -5,6 +5,7 @@ namespace Gplanchat\Durable\Activity; use Gplanchat\Durable\Awaitable\Awaitable; +use Gplanchat\Durable\Stub\StubArguments; /** * Proxy de planification côté workflow. @@ -55,14 +56,6 @@ public function __call(string $name, array $arguments): Awaitable */ private function argumentsToPayload(string $methodName, array $arguments): array { - $reflection = new \ReflectionMethod($this->contractClass, $methodName); - $params = $reflection->getParameters(); - $payload = []; - foreach ($params as $i => $param) { - $key = $param->getName(); - $payload[$key] = $arguments[$i] ?? ($param->isDefaultValueAvailable() ? $param->getDefaultValue() : null); - } - - return $payload; + return StubArguments::toPayload(new \ReflectionMethod($this->contractClass, $methodName), $arguments); } } diff --git a/src/Durable/Nexus/NexusStub.php b/src/Durable/Nexus/NexusStub.php index 4eed716c..5d4c2ff0 100644 --- a/src/Durable/Nexus/NexusStub.php +++ b/src/Durable/Nexus/NexusStub.php @@ -5,6 +5,7 @@ namespace Gplanchat\Durable\Nexus; use Gplanchat\Durable\Awaitable\Awaitable; +use Gplanchat\Durable\Stub\StubArguments; use Gplanchat\Durable\Nexus\Serving\NexusContractResolver; /** @@ -76,12 +77,6 @@ public function __call(string $name, array $arguments): Awaitable */ private function argumentsToPayload(string $methodName, array $arguments): array { - $payload = []; - foreach ((new \ReflectionMethod($this->contractClass, $methodName))->getParameters() as $i => $param) { - $payload[$param->getName()] = $arguments[$i] - ?? ($param->isDefaultValueAvailable() ? $param->getDefaultValue() : null); - } - - return $payload; + return StubArguments::toPayload(new \ReflectionMethod($this->contractClass, $methodName), $arguments); } } diff --git a/src/Durable/Stub/StubArguments.php b/src/Durable/Stub/StubArguments.php new file mode 100644 index 00000000..d521a486 --- /dev/null +++ b/src/Durable/Stub/StubArguments.php @@ -0,0 +1,87 @@ + $arguments tels que `__call` les a reçus : les positionnels + * sous des indices, les nommés sous leur nom + * + * @return array la charge nommée, un paramètre du contrat par clé + * + * @throws \BadMethodCallException si un argument nommé ne correspond à aucun paramètre + */ + public static function toPayload(\ReflectionFunctionAbstract $method, array $arguments): array + { + $payload = []; + $connus = []; + + foreach ($method->getParameters() as $i => $param) { + $name = $param->getName(); + $connus[$name] = true; + + if (\array_key_exists($i, $arguments)) { + $payload[$name] = $arguments[$i]; + + continue; + } + + if (\array_key_exists($name, $arguments)) { + $payload[$name] = $arguments[$name]; + + continue; + } + + $payload[$name] = $param->isDefaultValueAvailable() ? $param->getDefaultValue() : null; + } + + foreach ($arguments as $key => $_) { + if (\is_string($key) && !isset($connus[$key])) { + throw new \BadMethodCallException(\sprintf( + 'Unknown named parameter $%s for %s(); known parameters: %s.', + $key, + self::describe($method), + implode(', ', array_map(static fn (string $n): string => '$' . $n, array_keys($connus))), + )); + } + } + + return $payload; + } + + private static function describe(\ReflectionFunctionAbstract $method): string + { + return $method instanceof \ReflectionMethod + ? $method->getDeclaringClass()->getName() . '::' . $method->getName() + : $method->getName(); + } +} diff --git a/src/Durable/Workflow/ChildWorkflowStub.php b/src/Durable/Workflow/ChildWorkflowStub.php index db541a41..621b3ec9 100644 --- a/src/Durable/Workflow/ChildWorkflowStub.php +++ b/src/Durable/Workflow/ChildWorkflowStub.php @@ -5,6 +5,7 @@ namespace Gplanchat\Durable\Workflow; use Gplanchat\Durable\ChildWorkflowOptions; +use Gplanchat\Durable\Stub\StubArguments; /** * Proxy de planification côté workflow pour exécuter un workflow enfant typé. @@ -59,13 +60,6 @@ public function __call(string $name, array $arguments): \Gplanchat\Durable\Await */ private function argumentsToInput(array $arguments): array { - $params = $this->workflowMethod->getParameters(); - $input = []; - foreach ($params as $i => $param) { - $key = $param->getName(); - $input[$key] = $arguments[$i] ?? ($param->isDefaultValueAvailable() ? $param->getDefaultValue() : null); - } - - return $input; + return StubArguments::toPayload($this->workflowMethod, $arguments); } } diff --git a/tests/unit/Durable/Stub/StubArgumentsTest.php b/tests/unit/Durable/Stub/StubArgumentsTest.php new file mode 100644 index 00000000..e87a7b78 --- /dev/null +++ b/tests/unit/Durable/Stub/StubArgumentsTest.php @@ -0,0 +1,84 @@ + 'bonjour', 'times' => 3, 'tag' => 'défaut'], $this->map(['bonjour', 3])); + } + + /** + * Le défaut qui a coûté un après-midi : PHP passe les arguments nommés à `__call` dans un + * tableau à **clés de chaînes**. Appariés par indice, ils disparaissaient tous — et chaque + * paramètre retombait sur sa valeur par défaut, sans exception ni trace. + */ + public function testNamedArgumentsLandOnTheirParameter(): void + { + self::assertSame(['text' => 'bonjour', 'times' => 1, 'tag' => 'perso'], $this->map(['tag' => 'perso', 'text' => 'bonjour'])); + } + + public function testPositionalAndNamedArgumentsMix(): void + { + self::assertSame(['text' => 'bonjour', 'times' => 1, 'tag' => 'perso'], $this->map(['bonjour', 'tag' => 'perso'])); + } + + /** + * `??` confondait « absent » et « null » : passer explicitement `null` rendait la valeur par + * défaut, c'est-à-dire l'inverse de ce qui était demandé. + */ + public function testAnExplicitNullIsNotTheDefault(): void + { + self::assertNull($this->map(['text' => 'x', 'tag' => null])['tag']); + self::assertNull($this->map(['x', 1, null])['tag']); + } + + /** + * Une faute de frappe dans un nom d'argument ne doit pas être indiscernable d'une valeur par + * défaut voulue. PHP lève sur un appel ordinaire ; le stub aussi. + */ + public function testAnUnknownNamedArgumentIsRefused(): void + { + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessageMatches('/Unknown named parameter \$tags/'); + + $this->map(['text' => 'x', 'tags' => 'perso']); + } + + /** + * Un paramètre sans défaut et non fourni vaut `null` : c'est le comportement d'avant, et la + * validation reste au gestionnaire. + */ + public function testAMissingRequiredParameterIsNull(): void + { + self::assertNull($this->map([])['text']); + } + + /** + * @param array $arguments + * + * @return array + */ + private function map(array $arguments): array + { + $contract = new class { + public function greet(string $text, int $times = 1, ?string $tag = 'défaut'): void + { + } + }; + + return StubArguments::toPayload(new \ReflectionMethod($contract, 'greet'), $arguments); + } +} From 40fd98c991991bc7be365d9ffae5c62feaac84b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gr=C3=A9gory=20Planchat?= Date: Fri, 4 Sep 2026 01:58:34 +0200 Subject: [PATCH 2/2] =?UTF-8?q?fix(stubs):=20appliquer=20le=20principe=20e?= =?UTF-8?q?nti=C3=A8rement=20=E2=80=94=20trois=20fautes=20d'appel,=20trois?= =?UTF-8?q?=20refus?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit La relecture croisée n'a bloqué cette branche sur aucun axe, et a fait la même remarque huit fois : le principe est juste et il n'était appliqué qu'à moitié. La faute de frappe levait, les deux autres fautes du même ordre se taisaient. **Un paramètre requis non fourni valait `null`.** Silencieusement, y compris pour un type non nullable — donc la charge partait dans le journal, et le refus arrivait une passe de rejeu plus tard, dans un worker, loin de l'appel fautif. C'est exactement le mode de panne que cette branche corrigeait pour les arguments nommés : une faute qui voyage. PHP lève `ArgumentCountError` sur l'appel ordinaire correspondant. **Un paramètre servi en positionnel *et* en nommé** : le positionnel gagnait, sans un mot. PHP refuse (« Named parameter $x overwrites previous argument »), parce qu'il n'y a pas de bonne réponse à donner. Les deux lèvent désormais `\BadMethodCallException`, comme l'argument nommé inconnu. Le type diffère de celui de PHP — `\Error`, `\ArgumentCountError` — parce que l'appel passe par `__call` : c'est l'exception que la SPL réserve à une méthode appelée de travers, et elle reste rattrapable. Le docbloc de classe disait « PHP fait de même » ; il dit maintenant ce que PHP fait vraiment, et pourquoi le type retenu s'en écarte. Un paramètre variadique est passé : il n'a ni valeur par défaut ni obligation, il ne peut donc pas manquer, et il n'a pas de place à lui dans une charge nommée. `testAMissingRequiredParameterIsNull` codifiait le défaut — « c'est le comportement d'avant, et la validation reste au gestionnaire ». Il est remplacé par les deux cas qui lèvent, plus le cas voisin qui doit continuer de passer : un paramètre optionnel garde son défaut, un `null` explicite reste `null`. `UPGRADE.md` gagne la note qui manquait : trois comportements observables changent sur trois classes publiées. Elle dit aussi que ces exceptions sont déterministes — rejouées à l'identique à chaque redélivrance, elles brûlent les tentatives de Messenger jusqu'à un transport d'échec. Le formateur est repassé — c'est ce qui rendait la CS rouge sur les quatre versions de PHP. Co-Authored-By: Claude Opus 5 (1M context) --- UPGRADE.md | 29 ++++++++++ src/Durable/Nexus/NexusStub.php | 2 +- src/Durable/Stub/StubArguments.php | 57 +++++++++++++++---- tests/unit/Durable/Stub/StubArgumentsTest.php | 42 +++++++++++--- 4 files changed, 111 insertions(+), 19 deletions(-) diff --git a/UPGRADE.md b/UPGRADE.md index 074f7922..762b315a 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -22,6 +22,35 @@ 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é + +### Les stubs refusent ce que PHP refuse + +**Qui est concerné** : toute application qui appelle un contrat d'activité, d'opération Nexus ou de +workflow enfant par un stub. Rien à écrire ; des appels qui passaient en silence lèvent désormais, +et c'est le but. + +Les trois stubs transforment les arguments reçus par `__call` en une charge nommée. Trois fautes +d'appel y disparaissaient sans un mot, et voyageaient jusque dans le journal — où elles se rejouent +à l'identique, passe après passe, loin de l'appel fautif : + +| L'appel | Avant | Maintenant | Ce que PHP fait sur l'appel ordinaire | +|---|---|---|---| +| argument nommé inconnu | ignoré | `BadMethodCallException` | `Error: Unknown named parameter` | +| paramètre requis non fourni | vaut `null` | `BadMethodCallException` | `ArgumentCountError` | +| paramètre servi en positionnel **et** en nommé | le positionnel gagne | `BadMethodCallException` | `Error: Named parameter overwrites previous argument` | + +Le type est `\BadMethodCallException` et non celui de PHP parce que l'appel passe par `__call` : +c'est l'exception que la SPL réserve à une méthode appelée de travers, et elle reste rattrapable. + +**Si une de ces exceptions apparaît en production**, elle désigne un appel qui était déjà faux : un +workflow enfant démarré avec un paramètre manquant partait avec `null` et attendait un message qui +ne venait jamais. Rector ne peut rien ici — la correction est dans votre code d'appel, pas dans une +forme mécanique. + +Ces exceptions sont déterministes : rejouées à l'identique à chaque redélivrance, elles brûlent les +tentatives de Messenger jusqu'au transport d'échec. Configurez-en un. + ## 0.1.0-alpha8 ### Laravel refuse au démarrage un workflow dont les noms de paramètres divergent du contrat diff --git a/src/Durable/Nexus/NexusStub.php b/src/Durable/Nexus/NexusStub.php index 5d4c2ff0..6c46b52a 100644 --- a/src/Durable/Nexus/NexusStub.php +++ b/src/Durable/Nexus/NexusStub.php @@ -5,8 +5,8 @@ namespace Gplanchat\Durable\Nexus; use Gplanchat\Durable\Awaitable\Awaitable; -use Gplanchat\Durable\Stub\StubArguments; use Gplanchat\Durable\Nexus\Serving\NexusContractResolver; +use Gplanchat\Durable\Stub\StubArguments; /** * Proxy de planification côté appelant. diff --git a/src/Durable/Stub/StubArguments.php b/src/Durable/Stub/StubArguments.php index d521a486..23a2095f 100644 --- a/src/Durable/Stub/StubArguments.php +++ b/src/Durable/Stub/StubArguments.php @@ -22,15 +22,18 @@ * `null` à un paramètre nullable donnait sa valeur par défaut plutôt que `null` — c'est-à-dire * l'inverse de ce qui était demandé. D'où `array_key_exists` plutôt que `??`. * - * **Et un argument nommé inconnu lève**, au lieu de disparaître. PHP fait de même sur un appel - * ordinaire (`Unknown named parameter`) ; un stub qui l'avale rendrait une faute de frappe - * indiscernable d'une valeur par défaut voulue. + * **Et ce que PHP refuse, le stub le refuse.** Sur un appel ordinaire, PHP lève sur un argument + * nommé inconnu (`Unknown named parameter`), sur un argument requis manquant (`ArgumentCountError`) + * et sur un paramètre servi deux fois, en positionnel puis en nommé (`Named parameter $x overwrites + * previous argument`). Un stub qui avale l'un des trois rend une faute indiscernable d'une valeur + * voulue — et la fait voyager jusque dans le journal, où elle sera rejouée à l'identique. Les trois + * lèvent donc ici aussi. Le type diffère de celui de PHP — `\BadMethodCallException` plutôt que + * `\Error` ou `\ArgumentCountError` — parce que l'appel passe par `__call` : c'est l'exception que + * la SPL réserve à une méthode appelée de travers, et elle reste rattrapable. */ final class StubArguments { - private function __construct() - { - } + private function __construct() {} /** * @param array $arguments tels que `__call` les a reçus : les positionnels @@ -38,7 +41,9 @@ private function __construct() * * @return array la charge nommée, un paramètre du contrat par clé * - * @throws \BadMethodCallException si un argument nommé ne correspond à aucun paramètre + * @throws \BadMethodCallException si un argument nommé ne correspond à aucun paramètre, si un + * paramètre requis n'est pas fourni, ou si un paramètre est + * servi à la fois en positionnel et en nommé */ public static function toPayload(\ReflectionFunctionAbstract $method, array $arguments): array { @@ -49,19 +54,49 @@ public static function toPayload(\ReflectionFunctionAbstract $method, array $arg $name = $param->getName(); $connus[$name] = true; - if (\array_key_exists($i, $arguments)) { + // Un variadique n'a ni valeur par défaut ni obligation : il ne peut pas manquer, et + // il n'a pas de place à lui dans une charge nommée. + if ($param->isVariadic()) { + continue; + } + + $parPosition = \array_key_exists($i, $arguments); + $parNom = \array_key_exists($name, $arguments); + + if ($parPosition && $parNom) { + throw new \BadMethodCallException(\sprintf( + 'Parameter $%s of %s() was given both positionally and by name.', + $name, + self::describe($method), + )); + } + + if ($parPosition) { $payload[$name] = $arguments[$i]; continue; } - if (\array_key_exists($name, $arguments)) { + if ($parNom) { $payload[$name] = $arguments[$name]; continue; } - $payload[$name] = $param->isDefaultValueAvailable() ? $param->getDefaultValue() : null; + if ($param->isDefaultValueAvailable()) { + $payload[$name] = $param->getDefaultValue(); + + continue; + } + + // Le retomber-sur-`null` d'ici traversait le journal sans un mot, et se rejouait à + // l'identique à chaque passe : le paramètre est requis, son absence est une faute + // d'appel, et un type non nullable l'aurait de toute façon refusée à l'arrivée. + throw new \BadMethodCallException(\sprintf( + 'Missing required argument $%s for %s().', + $name, + self::describe($method), + )); } foreach ($arguments as $key => $_) { @@ -70,7 +105,7 @@ public static function toPayload(\ReflectionFunctionAbstract $method, array $arg 'Unknown named parameter $%s for %s(); known parameters: %s.', $key, self::describe($method), - implode(', ', array_map(static fn (string $n): string => '$' . $n, array_keys($connus))), + implode(', ', array_map(static fn(string $n): string => '$' . $n, array_keys($connus))), )); } } diff --git a/tests/unit/Durable/Stub/StubArgumentsTest.php b/tests/unit/Durable/Stub/StubArgumentsTest.php index e87a7b78..e8126520 100644 --- a/tests/unit/Durable/Stub/StubArgumentsTest.php +++ b/tests/unit/Durable/Stub/StubArgumentsTest.php @@ -58,12 +58,42 @@ public function testAnUnknownNamedArgumentIsRefused(): void } /** - * Un paramètre sans défaut et non fourni vaut `null` : c'est le comportement d'avant, et la - * validation reste au gestionnaire. + * Un paramètre requis non fourni lève, comme PHP lèverait `ArgumentCountError` sur l'appel + * ordinaire correspondant. + * + * Le laisser valoir `null` faisait voyager la faute jusque dans le journal, où elle se rejoue + * à l'identique à chaque passe : `$text` est déclaré `string`, une charge portant `null` est + * donc de toute façon refusée à l'arrivée — mais une passe de rejeu plus tard, dans un worker, + * loin de l'appel fautif. + */ + public function testUnParametreRequisNonFourniLeve(): void + { + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessageMatches('/Missing required argument \$text/'); + + $this->map([]); + } + + /** + * Servir le même paramètre en positionnel puis en nommé : PHP refuse (« Named parameter $x + * overwrites previous argument »), le stub choisissait le positionnel en silence. */ - public function testAMissingRequiredParameterIsNull(): void + public function testUnParametreServiDeuxFoisLeve(): void { - self::assertNull($this->map([])['text']); + $this->expectException(\BadMethodCallException::class); + $this->expectExceptionMessageMatches('/\$text.*both positionally and by name/'); + + $this->map([0 => 'positionnel', 'text' => 'nommé']); + } + + /** + * Le cas voisin, qui doit continuer de passer : un paramètre optionnel non fourni prend sa + * valeur par défaut, et un `null` explicite reste `null`. + */ + public function testUnParametreOptionnelGardeSonDefautEtAccepteNull(): void + { + self::assertSame(1, $this->map(['text' => 'x'])['times']); + self::assertNull($this->map(['text' => 'x', 'tag' => null])['tag']); } /** @@ -74,9 +104,7 @@ public function testAMissingRequiredParameterIsNull(): void private function map(array $arguments): array { $contract = new class { - public function greet(string $text, int $times = 1, ?string $tag = 'défaut'): void - { - } + public function greet(string $text, int $times = 1, ?string $tag = 'défaut'): void {} }; return StubArguments::toPayload(new \ReflectionMethod($contract, 'greet'), $arguments);