Skip to content

Add Temporal Nexus Operation Handler#2842

Open
Quinn-With-Two-Ns wants to merge 8 commits into
temporalio:masterfrom
Quinn-With-Two-Ns:temporal-nexus-operation-handler
Open

Add Temporal Nexus Operation Handler#2842
Quinn-With-Two-Ns wants to merge 8 commits into
temporalio:masterfrom
Quinn-With-Two-Ns:temporal-nexus-operation-handler

Conversation

@Quinn-With-Two-Ns
Copy link
Copy Markdown
Contributor

@Quinn-With-Two-Ns Quinn-With-Two-Ns commented Apr 14, 2026

Add TemporalOperationHandler for generic Nexus operations

Goal

Provide a new base to make it easy to expose more Temporal actions as Nexus Operations

Summary

  • Adds TemporalOperationHandler, a generic OperationHandler implementation that maps Nexus operations to Temporal workflows via a composable StartFunction
  • Adds TemporalNexusClient for starting workflows (typed and untyped) from within Nexus operation handlers
  • Supports subclassing TemporalOperationHandler to customize cancel behavior by overriding cancelWorkflowRun
  • Extracts shared workflow start logic into NexusStartWorkflowHelper, reused by both WorkflowRunOperation and the new handler

Test plan

  • GenericHandlerTypedStartWorkflowTest — typed workflow with return value
  • GenericHandlerTypedProcTest — typed void workflow
  • GenericHandlerUntypedStartWorkflowTest — untyped workflow start
  • GenericHandlerSyncResultTest — synchronous result path
  • GenericHandlerCancelTest — cancel with default and custom behavior

Note

Medium Risk
Adds new public Nexus handler/client APIs and changes operation-token parsing/cancel dispatch behavior, which could affect async operation lifecycle and cancellation semantics. Risk is moderate since changes are mostly additive but touch workflow start/link attachment and token validation used across Nexus operations.

Overview
Adds a new generic TemporalOperationHandler API that lets Nexus operation implementations return either sync results or start an async workflow via a new TemporalNexusClient (typed overloads + untyped) and TemporalOperationResult.

Refactors workflow-start/link attachment into NexusStartWorkflowHelper and updates WorkflowRunOperationImpl to use it.

Renames and generalizes workflow-run tokens to OperationToken, extends OperationTokenUtil with loadOperationToken (type-agnostic) while keeping a workflow-run asserting loader, and adds/updates tests covering typed/untyped starts, sync results, double-start guard, and cancel dispatch.

Reviewed by Cursor Bugbot for commit f2c65fc. Bugbot is set up for automated code reviews on this repo. Configure here.

@Quinn-With-Two-Ns Quinn-With-Two-Ns marked this pull request as ready for review April 14, 2026 22:33
@Quinn-With-Two-Ns Quinn-With-Two-Ns requested a review from a team as a code owner April 14, 2026 22:33
@Quinn-With-Two-Ns Quinn-With-Two-Ns force-pushed the temporal-nexus-operation-handler branch from a33ff01 to 7e61dab Compare May 8, 2026 14:14
throw new HandlerException(
HandlerException.ErrorType.INTERNAL,
new IllegalStateException("TemporalOperationResult must be either sync or async"));
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unreachable else branch is dead code

Low Severity

The else branch in TemporalOperationHandler.start() is unreachable dead code. TemporalOperationResult.isSync() returns the isSync field, and isAsync() returns !isSync — they are always complementary. No instance of TemporalOperationResult can ever be neither sync nor async, so the HandlerException at the bottom can never be thrown.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7e61dab. Configure here.

private final WorkflowClient client;
private final OperationContext operationContext;
private final OperationStartDetails operationStartDetails;
private boolean asyncOperationStarted;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgive my lack of Java concurrency know how, but could this flag cause a race if you call startWorkflow concurrently? Both attempt to start, race past the check and successfully start a workflow.

@Quinn-With-Two-Ns Quinn-With-Two-Ns force-pushed the temporal-nexus-operation-handler branch from 7e61dab to f2c65fc Compare May 15, 2026 23:04
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default mode and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

Reviewed by Cursor Bugbot for commit f2c65fc. Configure here.

// Set after successful start so that if startWorkflowAndAttachLinks throws,
// the handler can retry without being blocked by the guard.
asyncOperationStarted = true;
return TemporalOperationResult.async(response.getOperationToken());
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-thread-safe check-then-act on asyncOperationStarted flag

Medium Severity

The asyncOperationStarted boolean field uses a check-then-act pattern without synchronization. If startWorkflow is called concurrently from multiple threads sharing this client instance, both calls can race past the if (asyncOperationStarted) check before either sets the flag to true, resulting in two workflows being started when the invariant requires at most one. Making the field volatile alone wouldn't fix the TOCTOU issue; an AtomicBoolean with compareAndSet or synchronized block is needed to make the guard atomic.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f2c65fc. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants