Conversation
| /** | ||
| * Description of the workflow that backs a Nexus operation. | ||
| */ | ||
| final class WorkflowHandle |
There was a problem hiding this comment.
I was a little confused by the naming here as it felt a bit like this was a reference to a running workflow, but it's actually the input to a start workflow call.
There was a problem hiding this comment.
This mirrors java's io.temporal.nexus.WorkflowHandle naming
If you think that we should rename it, please suggest alternatives
…n timeouts Address PR #768 review: - deadlineFromHeaders parses Request-Timeout only (operation timeout is the total budget, left in forwarded headers); applied to cancel context too - add scheduleToStart and startToClose timeouts to NexusOperationOptions
Tasks 1-4 of the Nexus post-merge cleanup: - Add/sync the docs/ tree and CLAUDE.md to the committed handler model (drop deleted #[OperationCancel]/SynchronousOperationHandler references, fix the Nexus:: accessor table and #[AsyncOperation] return contract). - Remove comment-rule violations from branch-added Nexus files; expand abbreviations ($u/$m -> $uriParts/$pathMatches, etc.). - Home the shared endpoint/service override on ExecuteNexusOperationInput::effectiveOptions(), dropping the duplicated effectiveNexusOptions()/effectiveOptions() helpers. - Unify the empty endpoint/service error paths: drop the raw assert() and constructor validation so NexusOperationStub::assertOperationParams() is the single source of truth.
Task 5 — fix the NexusOperationStub start-handshake rejection: extract handleFromStarted() and, when the started-promise rejects, consume the result-promise so its rejection is not left unhandled. Add regression tests covering both the rejection surfacing and the no-leak guarantee. Task 6 — test-convention drift across tests/Nexus and tests/Unit/Nexus: expectExceptionMessageMatches -> full-message assertions, add missing #[CoversClass], and strip stray test-body comments.
Task 7 — add tests/Arch/NexusDependencyTest.php enforcing that Nexus\* depends only on Common/Exception/Workflow, with a verified exemption list for the bridge types that legitimately reach Client\*/Internal\* (the Handler/Internal dispatch subtree plus Nexus, Header, NexusOperationContext, WorkflowHandle). Document the exemption in CLAUDE.md.
Task 8 — verified the ErrorType wire values against nexus-rpc/sdk-go: the values are canonical, so no behavior change. Only document that Unknown is a local decode fallback for unrecognized incoming types (never emitted on the wire); every other case is a spec-defined value.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d85d80b05
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| $context->getClient()->send(new CommandResponse( | ||
| command: self::COMMAND_VALIDATED, | ||
| options: ['id' => $updateId], | ||
| payloads: null, | ||
| failure: null, |
There was a problem hiding this comment.
With the default JSON codec (WorkerFactory::createCodec() selects JsonCodec when RR_CODEC is unset), any workflow update now sends CommandResponse, but JsonCodec\Encoder has no CommandResponse branch and throws Unserializable command type ...CommandResponse. The previous UpdateResponse was the only type used here and was handled by the old protocol encoder; consequently update validation and completion fail for existing JSON-codec workers.
Useful? React with 👍 / 👎.
…n timeouts Address PR temporalio#768 review: - deadlineFromHeaders parses Request-Timeout only (operation timeout is the total budget, left in forwarded headers); applied to cancel context too - add scheduleToStart and startToClose timeouts to NexusOperationOptions
# Conflicts: # tests/Functional/ReplayerTestCase.php # tests/Unit/DTO/WorkflowOptionsTestCase.php
3 participants
What was changed
Samples: temporalio/samples-php#75
Why?
Checklist
Closes
How was this tested: