Add Standalone Activities to Temporal Nexus Operation Handler#2918
Add Standalone Activities to Temporal Nexus Operation Handler#2918Quinn-With-Two-Ns wants to merge 4 commits into
Conversation
| * activity is scheduled from a Nexus operation handler. | ||
| */ | ||
| @Experimental | ||
| final class CompletionCallback { |
There was a problem hiding this comment.
Need to think if this is the right way to propogate this information
aa093f1 to
441ee33
Compare
![chatgpt-codex-connector[bot]](https://cdn.github.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: 441ee33b6c
ℹ️ 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".
| activityType, | ||
| args, | ||
| options, | ||
| Header.empty(), |
There was a problem hiding this comment.
When a Nexus handler starts an activity through this new path, any ContextPropagators configured on the surrounding WorkflowClient are lost because the StartActivityInput is always built with Header.empty(). Standalone ActivityClient.start normally serializes those propagators into the header before reaching the invoker, so callers relying on MDC/tracing/auth context propagation will see backing activities started from Nexus run without that context; build the header from client.getOptions().getContextPropagators() or route through the public ActivityClient.start path instead.
Useful? React with 👍 / 👎.
VegetarianOrc
left a comment
VegetarianOrc
left a comment
There was a problem hiding this comment.
Looks good to me overall. Just want to make sure we get the token type enum aligned with the other SDKs and to confirm if we want the response link in this PR.
| // TODO: Attach activity-event link when server-side activity link support is available. | ||
| // No StartActivityResponseLink analog exists and there is no verified activity-event link path | ||
| // in LinkConverter. Do NOT copy the synthetic workflow-event link fabrication from | ||
| // NexusStartWorkflowHelper; Nexus operations function correctly without diagnostic links. |
There was a problem hiding this comment.
Should we implement this TODO as part of this PR? The API seems to return the response link: https://github.com/temporalio/api/blob/96d4995096d1589e035104356f6cdd4fa799dc36/temporal/api/workflowservice/v1/request_response.proto#L3257
| // Values 2 and 3 are reserved for future token types (e.g. update-workflow, get-workflow-result). | ||
| ACTIVITY_EXECUTION(4); |
There was a problem hiding this comment.
Activity should be 2 and workflow update should be 3
There was a problem hiding this comment.
Super minor nit: This test file now contains tests for more than just WorkflowRunToken
| } | ||
|
|
||
| @Test | ||
| public void testActivityOperationTokenRoundTrip() throws Exception { |
There was a problem hiding this comment.
This test looks like it might be duplicated in the WorkflowRunTokenTest file?
![cursor[bot]](https://cdn.github.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit 4706130. Configure here.
| WORKFLOW_RUN(1); | ||
| WORKFLOW_RUN(1), | ||
| // Values 2 and 3 are reserved for future token types (e.g. update-workflow, get-workflow-result). | ||
| ACTIVITY_EXECUTION(4); |
There was a problem hiding this comment.
Wrong activity token type
High Severity
ACTIVITY_EXECUTION is assigned numeric type 4, while the intended cross-SDK discriminator for standalone activity tokens is 2. Tokens emitted by this SDK won’t match other Temporal Nexus implementations or cancel dispatch that expects type 2.
Reviewed by Cursor Bugbot for commit 4706130. Configure here.
| nexusOperationMetadata.callbackHeaders, | ||
| nexusOperationMetadata.operationToken, | ||
| protoLinks); | ||
| request.addCompletionCallbacks(cb); |
There was a problem hiding this comment.
Empty callback still registered
Medium Severity
When Nexus operation metadata is present, startActivity always attaches a Nexus completion callback, even if callbackUrl is empty. The workflow start path only registers callbacks when the URL is non-empty, so this can send invalid completion callbacks on StartActivityExecution.
Reviewed by Cursor Bugbot for commit 4706130. Configure here.
2 participants
What was changed
Why?
Checklist
Closes
How was this tested:
Note
Medium Risk
Touches Nexus completion callbacks, operation tokens, and activity scheduling RPCs with subtle pre/post-RPC token differences; behavior is largely patterned on the existing workflow Nexus path but the new surface is large and marked experimental.
Overview
Nexus operation handlers can now back async operations with standalone activity executions, parallel to the existing workflow-run path.
TemporalNexusClientgains typed and untypedstartActivityoverloads that return an activity-execution operation token; the same one-async-operation-per-handler guard applies across workflow and activity starts.The activity path routes
StartActivityInputthroughActivityClientInternal.getInvoker()so interceptors and metrics stay intact.RootActivityClientInvokerreadsNexusOperationMetadatafrom thread-local Nexus context to set the Nexus request ID, inbound links, completion callbacks (via sharedInternalUtils.buildNexusCallback), and a pre-RPC activity operation token in callback headers; it also recordsStartActivityExecutionResponselinks on the operation context. The token returned to the Nexus caller is regenerated with run ID after the start RPC, while the callback header token omits run ID.OperationTokenType.ACTIVITY_EXECUTION(t:4) andaid/ridfields support cancel dispatch:TemporalOperationHandlerdefaults toActivityClientcancel via overridablecancelActivityExecution.LinkConverteradds activity link URL round-trips. Nexus callback header injection for workflows is refactored intobuildNexusCallbackwithout behavior change.Reviewed by Cursor Bugbot for commit 4706130. Bugbot is set up for automated code reviews on this repo. Configure here.