Generic Temporal Nexus Operation Handler#690
Conversation
jmaeagle99
left a comment
There was a problem hiding this comment.
Still reviewing but wanted to leave my initial comments.
| /// <para>WARNING: Nexus support is experimental.</para> | ||
| /// <para>This class supports inheritance to customize cancel behavior. Override | ||
| /// <see cref="CancelWorkflowRunAsync"/> to change how workflow-run cancellations are handled. | ||
| /// The <see cref="StartAsync"/> and <see cref="CancelAsync"/> methods should not be |
There was a problem hiding this comment.
These methods cannot be overridden because they are not virtual. I would drop this comment.
There was a problem hiding this comment.
If there is some specific concern that derivations of the class will shadow these methods (and callers are not directly invoking these methods via the TemporalNexusOperationHandler class itself), then make them explicit interface implementations e.g. async Task<OperationStartResult<TResult>> IOperationHandler<TInput, TResult>.StartAsync(...)
There was a problem hiding this comment.
If you want to keep it to doc comments, I would suggest adding these to each of the methods instead. They should compose with the inheritdoc, especially since the interface methods do not use <remarks>.
There was a problem hiding this comment.
Ah okay I think I was a bit confused about dotnets behaviour if a user made a subclass
| /// Gets the synchronous result value. Only meaningful when <see cref="IsSyncResult"/> is | ||
| /// true. | ||
| /// </summary> | ||
| internal TResult? SyncValue { get; } |
There was a problem hiding this comment.
Rather than these be auto-generated properties, should we stored them in backing fields and have the properties throw an exception if IsSyncResult is not the expected state?
There was a problem hiding this comment.
Sure can do
| /// <param name="workflowId">The workflow ID extracted from the operation token.</param> | ||
| /// <returns>Task for cancel completion.</returns> | ||
| protected virtual Task CancelWorkflowRunAsync( | ||
| OperationCancelContext context, string workflowId) => |
There was a problem hiding this comment.
The context parameter is not used at all in this default implementation. Is this desired? How does user cancellation flow into the cancel gesture?
There was a problem hiding this comment.
these are run by the worker so users don't cancel these methods and the server doesn't have a way cancel these method either. It is assumed they are fast (<10s)
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 9eb951c. Configure here.

What was changed
Added a generic Nexus operation handler that consolidates the common pattern of "either return a synchronous
result or start a workflow as an async operation."
Why?
The existing
WorkflowRun/SyncOperationsplit forces users to pick the operation shape up front, which is awkward when:The new TemporalOperation unifies these cases behind one API and makes the common path much shorter.
Checklist
Closes
How was this tested:
Note
Medium Risk
Touches experimental Nexus start/cancel and callback/token paths shared with existing workflow-run handlers; behavior is well covered by new tests but affects cross-service operation semantics.
Overview
Introduces
TemporalNexusOperationHandler.FromHandleFactory, a single Nexus handler path where the start delegate returnsTemporalOperationResult<TResult>—either an immediate value or an async workflow-run token—instead of choosing up front between workflow-run and sync handler shapes.Handlers receive
ITemporalNexusClient/TemporalNexusClientto start workflows (with links, callbacks, request ID, andNexus-Operation-Tokenheader wiring) or useTemporalClientfor sync work like signals. Workflow-start plumbing is centralized inNexusWorkflowStartHelper;WorkflowRunOperationContextnow delegates to it.NexusWorkflowRunHandlegainsParseToken, typedOperationToken, and cancel routing by token type with overridableCancelWorkflowRunAsync.Unit and worker integration tests cover tokens, sync/async results, cancel behavior, links, conflict policy, and no-input overloads.
Reviewed by Cursor Bugbot for commit ab83e06. Bugbot is set up for automated code reviews on this repo. Configure here.