Propagate Nexus links and request IDs to all activity starts in Nexus handlers - #3048
Open
tekkaya wants to merge 2 commits into
Open
Propagate Nexus links and request IDs to all activity starts in Nexus handlers#3048tekkaya wants to merge 2 commits into
tekkaya wants to merge 2 commits into
Conversation
tekkaya
force-pushed
the
nexus-saa-bypass-links-dedup
branch
2 times, most recently
from
August 28, 2026 20:10
eddaa54 to
507ea44
Compare
tekkaya
marked this pull request as ready for review
August 28, 2026 20:46
tekkaya
force-pushed
the
nexus-saa-bypass-links-dedup
branch
from
August 29, 2026 04:55
507ea44 to
9377346
Compare
tekkaya
changed the title
tekkaya
changed the title
tekkaya
force-pushed
the
nexus-saa-bypass-links-dedup
branch
from
August 29, 2026 05:02
9377346 to
0d6c817
Compare
tekkaya
changed the title
tekkaya
force-pushed
the
nexus-saa-bypass-links-dedup
branch
from
August 31, 2026 22:24
0d6c817 to
030ecb7
Compare
tekkaya
changed the title
tekkaya
changed the title
…on handler TemporalNexusClient guards its own activity/workflow/update start to at most one per operation invocation, so a synchronous Nexus operation handler that starts two or more activities inline has to bypass it and use a raw ActivityClient obtained from Nexus.getOperationContext() instead. That bypass path got no request links, because RootActivityClientInvoker.startActivity derived link attachment, on-conflict dedup, and completion-callback attachment all from the same NexusOperationMetadata, which only the guarded TemporalNexusClient call ever sets. Every activity start made during the invocation now reuses the inbound Nexus task's request ID and gets its inbound links, regardless of which client object issued it. NexusOperationMetadata keeps its narrow, one-shot scope and remains the only thing that can attach a completion callback, since only the guarded start should complete the Nexus operation. When metadata is present its own request ID still takes precedence, so the guarded start's identity never depends on the ambient value also having been set. Reusing one ambient ID across every start in an invocation is a deliberate, known tradeoff: a handler that starts a fresh run under an activity ID it already used earlier in the same invocation can have that start incorrectly resolve to the stale run instead of creating a new one. Two narrower, redelivery-aware alternatives were tried and dropped -- raw ambient-ID reuse scoped to only the guarded call, then a per-call ID derived from each start's ordinal position -- because both require assuming the handler reissues an identical sequence of calls on every retry, an assumption the SDK has no way to verify. This change instead matches sdk-go's approved fix (temporalnexus/temporal_operation.go, PR #2633) and sdk-python's current behavior (temporalio/nexus/_operation_context.py): every activity start in a Nexus context reuses the ambient request ID unconditionally. sdk-python's own attempt at the narrower, backing-call-only scoping (PR #1722) was closed without merging for the same reason. ActivityOperationLinkingTest (functional, requires a real server) drives a synchronous handler that starts two bypass-path activities and asserts both the forward link (each activity's own ActivityExecutionInfo) and the backward links (both activities' completions landing on the caller's single NexusOperationCompleted event), the same way SignalOperationLinkingTest already does for signals. RootActivityClientInvokerTest covers metadata's request ID taking precedence over the ambient one, the ambient-links-and-request-ID-without-metadata case, the outside-Nexus-context case, and two bypass-path starts in one invocation sharing the ambient request ID.
tekkaya
force-pushed
the
nexus-saa-bypass-links-dedup
branch
from
August 31, 2026 22:36
030ecb7 to
8fdbb33
Compare
The server rejects a StartActivityExecutionRequest whose OnConflictOptions sets attach_request_id when the request carries neither a link nor a completion callback (chasm/lib/activity/validator.go's validateOnConflictOptions: "attach_request_id requires at least one completion callback or link"). The previous code set attach_request_id and attach_links unconditionally whenever any Nexus context existed, regardless of whether the inbound task actually had links -- a bypass-path activity start issued during an invocation whose inbound Nexus task carries no links would send exactly that invalid combination and get rejected. This wasn't caught before because the existing unit tests mock the client and never exercise real server-side validation. OnConflictOptions is now only set when there's something to attach, and each flag reflects what the request actually carries, matching sdk-go's identical gating in its own fix (temporalnexus/temporal_operation.go, PR #2633). This also fixes the guarded (metadata-backed) path: it previously set attach_completion_callbacks based on whether metadata was present rather than whether a callback URL was actually set, so a guarded start with an empty callback URL and no links would hit the same rejection. RootActivityClientInvokerTest: flipped the assertion in nexusMetadataWithEmptyCallbackUrlOmitsCompletionCallback (attach_completion_ callbacks now correctly reflects the absence of a real callback), rewrote nexusContextWithoutAmbientStateStartsOrdinaryActivity to assert OnConflictOptions is entirely absent, and added metadataWithEmptyCallbackUrlAndNoLinksOmitsOnConflictOptions covering the previously-untested guarded-call variant of the same bug.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was changed
RootActivityClientInvoker.startActivitynow propagates the inbound Nexus task’s links and request ID to every activity start made on the operation-handler thread, including starts made through a rawActivityClient.NexusOperationMetadataremains narrowly scoped to the guarded backing start: it is the only mechanism that can attach a completion callback, because only that start is allowed to complete the Nexus operation. When present, its request ID takes precedence over the ambient inbound request ID.Why?
TemporalNexusClientpermits only one guarded primitive activity start per operation invocation. A synchronous handler that starts another activity must use a rawActivityClient; previously, those additional activity starts received neither inbound links nor redelivery-safe request-ID reuse.Sharing the inbound request ID across all starts is deliberate. A handler that reuses an activity ID within one invocation can resolve a later start to the earlier run; deriving per-start IDs would require assuming the handler repeats starts in the same order after redelivery.
Restricting request-ID reuse to only the guarded start preserves the previous behavior, but does not address additional starts made through a raw
ActivityClient. This approach aligns with the approved sdk-go fix #2633.Checklist
Closes the multiple-activity-start gap identified during Java SDK testing of Nexus + Standalone Activities.
Tested with:
RootActivityClientInvokerTest(unit)ActivityOperationLinkingTest(functional; requires a real server and is gated bySDKTestWorkflowRule.useExternalService)Documentation updates needed: No.