fix(workflow): a query handler, an update validator, an await condition and a side effect callback can no longer create a command

src/Interceptor/WorkflowOutboundCalls/TimerInput.php

@@ -24,7 +24,6 @@ final class TimerInput
      */
     public function __construct(
         public readonly \DateInterval $interval,
-
         public readonly ?TimerOptions $timerOptions,
     ) {}
 

src/Internal/Workflow/Process/Process.php

@@ -55,17 +55,16 @@ public function __construct(
         $workflowInstance->getQueryDispatcher()
             ->setQueryExecutor(function (QueryInput $input, callable $handler) use ($inboundPipeline): mixed {
                 try {
-                    return $inboundPipeline->with(
+                    return $this->scopeContext->runReadOnly(fn(): mixed => $inboundPipeline->with(
                         function (QueryInput $input) use ($handler): mixed {
-                            $context = $this->scopeContext
-                                ->withInput(new Input($this->scopeContext->getInfo(), $input->arguments));
-                            $context->setReadonly(true);
-                            Workflow::setCurrentContext($context);
+                            Workflow::setCurrentContext($this->scopeContext
+                                ->withInput(new Input($this->scopeContext->getInfo(), $input->arguments)));
+
                             return $handler($input->arguments);
                         },
                         /** @see WorkflowInboundCallsInterceptor::handleQuery() */
                         'handleQuery',
-                    )($input);
+                    )($input));
                 } finally {
                     Workflow::setCurrentContext(null);
                 }
@@ -76,7 +75,7 @@ function (QueryInput $input) use ($handler): mixed {
             ->setUpdateValidator(function (UpdateInput $input, callable $handler) use ($inboundPipeline): void {
                 try {
                     Workflow::setCurrentContext($this->scopeContext);
-                    $inboundPipeline->with(
+                    $this->scopeContext->runReadOnly(fn(): mixed => $inboundPipeline->with(
                         function (UpdateInput $input) use ($handler): void {
                             Workflow::setCurrentContext($this->scopeContext->withInput(
                                 new Input(
@@ -89,7 +88,7 @@ function (UpdateInput $input) use ($handler): void {
                         },
                         /** @see WorkflowInboundCallsInterceptor::validateUpdate() */
                         'validateUpdate',
-                    )($input);
+                    )($input));
                 } finally {
                     Workflow::setCurrentContext(null);
                 }

src/Internal/Workflow/ScopeContext.php

@@ -14,6 +14,7 @@
 use React\Promise\Deferred;
 use React\Promise\PromiseInterface;
 use Temporal\Exception\Failure\CanceledFailure;
+use Temporal\Exception\IllegalStateException;
 use Temporal\Internal\Transport\CompletableResult;
 use Temporal\Internal\Workflow\Process\Scope;
 use Temporal\Worker\Transport\Command\RequestInterface;
@@ -55,20 +56,38 @@ public static function fromWorkflowContext(
         $ctx->onRequest = $onRequest;
         $ctx->updateContext = $updateContext;
         $ctx->readonly = $context->readonly;
+        /** @psalm-suppress UnsupportedPropertyReferenceUsage */
+        $ctx->inReadOnlyCallback = &$context->inReadOnlyCallback;
         $ctx->continueAsNew = $context->continueAsNew;
         $ctx->trace = &$context->trace;
         $ctx->currentDetails = &$context->currentDetails;
 
         return $ctx;
     }
 
+    #[\Override]
+    public function assertWritable(): void
+    {
+        if ($this->inReadOnlyCallback) {
+            throw new IllegalStateException(
+                'Workflow calls that send commands are not allowed inside read-only callbacks.',
+            );
+        }
+
+        $this->parent->assertWritable();
+    }
+
     public function async(callable $handler): CancellationScopeInterface
     {
+        $this->assertWritable();
+
         return $this->scope->startScope($handler, false);
     }
 
     public function asyncDetached(callable $handler): CancellationScopeInterface
     {
+        $this->assertWritable();
+
         return $this->scope->startScope($handler, true);
     }
 
@@ -78,6 +97,8 @@ public function request(
         bool $cancellable = true,
         bool $waitResponse = true,
     ): PromiseInterface {
+        $this->assertWritable();
+
         $cancellable && $this->scope->isCancelled() && throw new CanceledFailure(
             'Attempt to send request to cancelled scope',
         );

src/Internal/Workflow/WorkflowContext.php

@@ -17,6 +17,7 @@
 use React\Promise\Deferred;
 use React\Promise\Exception\LengthException;
 use React\Promise\PromiseInterface;
+use Temporal\Exception\IllegalStateException;
 use Temporal\Activity\ActivityOptions;
 use Temporal\Activity\ActivityOptionsInterface;
 use Temporal\Activity\LocalActivityOptions;
@@ -71,6 +72,7 @@
 use Temporal\Promise;
 use Temporal\Worker\FeatureFlags;
 use Temporal\Worker\Transport\Command\RequestInterface;
+use Temporal\Workflow;
 use Temporal\Workflow\ActivityStubInterface;
 use Temporal\Workflow\ChildWorkflowOptions;
 use Temporal\Workflow\ChildWorkflowStubInterface;
@@ -102,6 +104,7 @@ class WorkflowContext implements WorkflowContextInterface, HeaderCarrier, Destro
     protected array $trace = [];
     protected bool $continueAsNew = false;
     protected bool $readonly = true;
+    protected bool $inReadOnlyCallback = false;
     protected ?string $currentDetails = null;
 
     /** @var Pipeline<WorkflowOutboundRequestInterceptor, PromiseInterface> */
@@ -176,10 +179,50 @@ public function setReadonly(bool $value = true): static
         return $this;
     }
 
+    public function isReadonly(): bool
+    {
+        return $this->readonly || $this->inReadOnlyCallback;
+    }
+
+    public function assertWritable(): void
+    {
+        if ($this->inReadOnlyCallback) {
+            throw new IllegalStateException(
+                'Workflow calls that send commands are not allowed inside read-only callbacks.',
+            );
+        }
+
+        if ($this->readonly) {
+            throw new \RuntimeException('Workflow is not initialized.');
+        }
+    }
+
+    /**
+     * @template T
+     * @param callable(): T $callback
+     * @return T
+     */
+    public function runReadOnly(callable $callback): mixed
+    {
+        if ($this->inReadOnlyCallback || !FeatureFlags::$readOnlyWorkflowCallbacks) {
+            return $callback();
+        }
+
+        $this->inReadOnlyCallback = true;
+
+        try {
+            return $callback();
+        } finally {
+            $this->inReadOnlyCallback = false;
+        }
+    }
+
     public function withInput(Input $input): static
     {
         $clone = clone $this;
         $clone->awaits = &$this->awaits;
+        /** @psalm-suppress UnsupportedPropertyReferenceUsage */
+        $clone->inReadOnlyCallback = &$this->inReadOnlyCallback;
         $clone->trace = &$this->trace;
         $clone->input = $input;
         return $clone;
@@ -264,11 +307,13 @@ public function sideEffect(callable $context, ?SideEffectOptions $options = null
 
         try {
             if (!$this->isReplaying()) {
-                $value = $this->callsInterceptor->with(
-                    $closure,
-                    /** @see WorkflowOutboundCallsInterceptor::sideEffect() */
-                    'sideEffect',
-                )(new SideEffectInput($closure, $options));
+                $value = $this->runReadOnly(
+                    fn(): mixed => $this->callsInterceptor->with(
+                        $closure,
+                        /** @see WorkflowOutboundCallsInterceptor::sideEffect() */
+                        'sideEffect',
+                    )(new SideEffectInput($closure, $options)),
+                );
             }
         } catch (\Throwable $e) {
             return reject($e);
@@ -491,7 +536,7 @@ public function request(
         bool $cancellable = true,
         bool $waitResponse = true,
     ): PromiseInterface {
-        $this->readonly and throw new \RuntimeException('Workflow is not initialized.');
+        $this->assertWritable();
         $this->recordTrace();
 
         // Intercept workflow outbound calls
@@ -672,7 +717,7 @@ public function resolveConditions(): void
     {
         foreach ($this->awaits as $awaitsGroupId => $awaitsGroup) {
             foreach ($awaitsGroup as $i => [$condition, $deferred]) {
-                if ($condition()) {
+                if ($this->runReadOnly($condition)) {
                     unset($this->awaits[$awaitsGroupId][$i]);
                     $deferred->resolve(null);
                     $this->resolveConditionGroup($awaitsGroupId);
@@ -776,7 +821,9 @@ protected function awaitRequest(callable|Mutex|PromiseInterface ...$conditions):
             $condition instanceof Mutex and $condition = static fn(): bool => !$condition->isLocked();
 
             if ($condition instanceof \Closure) {
-                $callableResult = $condition($conditionGroupId);
+                $callableResult = $this->runReadOnly(
+                    static fn(): mixed => $condition($conditionGroupId),
+                );
                 if ($callableResult === true) {
                     $this->resolveConditionGroup($conditionGroupId);
                     return resolve(true);
@@ -846,6 +893,10 @@ protected function addCondition(string $conditionGroupId, callable $condition):
      */
     protected function recordTrace(): void
     {
-        $this->readonly or $this->trace = \debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS);
+        if ($this->isReadonly()) {
+            return;
+        }
+
+        $this->trace = \debug_backtrace(\DEBUG_BACKTRACE_IGNORE_ARGS);
     }
 }

src/Worker/FeatureFlags.php

@@ -89,4 +89,15 @@ final class FeatureFlags
      * @link https://github.com/temporalio/sdk-php/issues/399
      */
     public static bool $settleAwaitOnFirstSettledCondition = false;
+
+    /**
+     * Reject a call that sends a command from a callback that only observes the workflow: a query
+     * handler, an update validator, an await condition or a {@see Workflow::sideEffect()} callback.
+     * Such a command does not belong to the workflow's own sequence and breaks replay.
+     *
+     * Set to FALSE to keep the previous behavior, where the command is created.
+     *
+     * @since SDK 2.18.0
+     */
+    public static bool $readOnlyWorkflowCallbacks = true;
 }

testing/src/TestService.php

@@ -20,14 +20,20 @@
 final class TestService
 {
     private TestServiceClient $testServiceClient;
-
     private int $lockDelta = 0;
 
     public function __construct(TestServiceClient $testServiceClient)
     {
         $this->testServiceClient = $testServiceClient;
     }
 
+    public static function create(string $host): self
+    {
+        return new self(
+            new TestServiceClient($host, ['credentials' => ChannelCredentials::createInsecure()]),
+        );
+    }
+
     /**
      * Net lock/unlock delta applied through this instance since it was created.
      *
@@ -39,13 +45,6 @@ public function lockDelta(): int
         return $this->lockDelta;
     }
 
-    public static function create(string $host): self
-    {
-        return new self(
-            new TestServiceClient($host, ['credentials' => ChannelCredentials::createInsecure()]),
-        );
-    }
-
     /**
      * Increments Time Locking Counter by one.
      *

testing/src/WorkflowTestCase.php

@@ -50,6 +50,19 @@ protected function tearDown(): void
         parent::tearDown();
     }
 
+    /**
+     * @return list<Interceptor>
+     */
+    protected function clientInterceptors(): array
+    {
+        return [];
+    }
+
+    protected function interactions(WorkflowRunInterface $run): WorkflowInteractions
+    {
+        return WorkflowInteractions::of($this->workflowClient, $run);
+    }
+
     private function assertTimeSkippingBalanced(): void
     {
         $delta = $this->testingService->lockDelta();
@@ -71,17 +84,4 @@ private function assertTimeSkippingBalanced(): void
             $delta,
         ));
     }
-
-    /**
-     * @return list<Interceptor>
-     */
-    protected function clientInterceptors(): array
-    {
-        return [];
-    }
-
-    protected function interactions(WorkflowRunInterface $run): WorkflowInteractions
-    {
-        return WorkflowInteractions::of($this->workflowClient, $run);
-    }
 }

tests/Unit/Internal/Support/DateIntervalTestCase.php

@@ -201,7 +201,6 @@ public function testParseDetectsIso8601FormatCorrectly(string $interval, bool $s
         // Arrange
         $reflection = new \ReflectionClass(DateInterval::class);
         $method = $reflection->getMethod('isIso8601DurationFormat');
-        $method->setAccessible(true);
 
         // Act
         $result = $method->invoke(null, $interval);