Conversation
83f4524 to
298145d
Compare
Add SerializationContext support so a payload converter or failure converter can know which namespace, workflow, or activity a payload belongs to: - SerializationContext marker with WorkflowSerializationContext and ActivitySerializationContext, plus SerializationContextAwareInterface and SerializationContextApplier for opt-in context binding. - Outbound workflow context is resolved at a single choke point in WorkflowContext::request per command type (activity, local activity, child workflow, external signal, complete, side effect, continue-as-new); stubs create plain EncodedValues and carry no context. - Inbound routers, client API (start, signal-with-start, update-with-start, query, update, result), start memo, schedule mapping, pending activity info, heartbeat details, and failure chains are converted with the proper context. - ActivityCompletionClient stays context-free by default with an explicit withContext escape hatch for out-of-band completion. Covered by unit tests and an acceptance suite that signs every payload with its serialization context and rejects any mismatch.
298145d to
46a74b8
Compare
![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: 46a74b8ce1
ℹ️ 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".
| if ($this->values !== null) { | ||
| $converter = $this->converter(); |
There was a problem hiding this comment.
When EncodedValues::empty() or EncodedValues::fromValues([]) has no converter, toPayloads() now calls converter() before discovering that there are no values and throws LogicException. Previously the empty loop returned an empty Payloads message without requiring a converter, so callers that serialize an empty collection directly regress; return early for an empty values array or obtain the converter only inside the loop.
Useful? React with 👍 / 👎.
| $collection = $memo === null | ||
| ? EncodedCollection::fromValues([], $this->converter) | ||
| : EncodedCollection::fromPayloadCollection($memo->getFields(), $this->converter); | ||
| $collection->setSerializationContext($this->context); |
There was a problem hiding this comment.
When results come through WorkflowClient::listWorkflowExecutions(), the mapper is constructed without a context, so this assigns null to every memo even though the response contains each workflow ID and the list request knows the namespace. Accessing a memo encoded by a context-aware converter will therefore fail or use the wrong key, while describe() works because it supplies a context; construct a workflow context per mapped execution for list results.
Useful? React with 👍 / 👎.
| $context = $this->getSerializationContext(); | ||
|
|
||
| $queryArguments = EncodedValues::fromValues($args, $converter); | ||
| $queryArguments->setSerializationContext($context); |
There was a problem hiding this comment.
If a client query interceptor uses the supported QueryInput::with(workflowExecution: ...) API to route the query to another workflow, this context was captured from the original stub before the interceptor runs. The request arguments remain signed for the original workflow and the returned payload is also decoded under that original context, so context-aware converters fail for the rerouted query; derive and apply the context from the final $input->workflowExecution inside the terminal callback.
Useful? React with 👍 / 👎.
2 participants
What was changed
Why?
Closes #587
Checklist
Closes
How was this tested: