Only send request ID when in a backing nexus context when starting a SAA - #1722
Closed
VegetarianOrc wants to merge 3 commits into
Closed
Only send request ID when in a backing nexus context when starting a SAA#1722VegetarianOrc wants to merge 3 commits into
VegetarianOrc wants to merge 3 commits into
Conversation
THardy98
reviewed
Aug 5, 2026
THardy98
left a comment
THardy98
left a comment
Contributor
There was a problem hiding this comment.
Could we add a test for this case:
Sending request IDs for every SAA prevents starting multiple from the Nexus handler.
That should fail prior to this change and fixed post-change
VegetarianOrc
force-pushed
the
amazzeo/nexus-saa-request-id
branch
from
August 20, 2026 20:17
ec84128 to
a7c37da
Compare
Contributor
Author
Added! |
tekkaya
added a commit
to temporalio/sdk-java
that referenced
this pull request
Aug 29, 2026
…t-ID scoping 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 gets the inbound Nexus request's 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 or a reused request ID -- an earlier attempt at this fix also gave bypass-path activity starts the invocation's ambient request ID (first raw, then via a per-call derived counter to avoid collisions between two starts sharing an activity ID), but both versions were dropped: correctly identifying a start as "the same call, redelivered" vs. "a new call that happens to land on the same ordinal position" requires assuming the handler reissues an identical sequence of calls on every retry, an assumption the SDK has no way to verify. Under non-determinism this can silently fail to dedup a genuinely redelivered start, reintroducing the very duplicate-execution bug the reuse was meant to prevent. sdk-python reached the same conclusion independently (temporalio/sdk-python#1722, "Only send request ID when in a backing nexus context when starting a SAA") and made the same choice: only the one guarded backing start reuses the Nexus task's request ID; every bypass-path start gets its own fresh, uncorrelated one. This change aligns Java with that. 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. It also covers the same-activity-ID restart pattern from sdk-python's regression test: starting a fresh run with an already-used activity ID within one invocation must not resolve to the stale, already-completed run. RootActivityClientInvokerTest covers the ambient-links-but-fresh-request-ID case, the outside-Nexus-context case, and two bypass-path starts for the same activity ID getting distinct request IDs, at the unit level.
tekkaya
added a commit
to temporalio/sdk-java
that referenced
this pull request
Aug 29, 2026
…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 gets the inbound Nexus request's 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 or a reused request ID -- an earlier attempt at this fix also gave bypass-path activity starts the invocation's ambient request ID (first raw, then via a per-call derived counter to avoid collisions between two starts sharing an activity ID), but both versions were dropped: correctly identifying a start as "the same call, redelivered" vs. "a new call that happens to land on the same ordinal position" requires assuming the handler reissues an identical sequence of calls on every retry, an assumption the SDK has no way to verify. Under non-determinism this can silently fail to dedup a genuinely redelivered start, reintroducing the very duplicate-execution bug the reuse was meant to prevent. sdk-python reached the same conclusion independently (temporalio/sdk-python#1722, "Only send request ID when in a backing nexus context when starting a SAA") and made the same choice: only the one guarded backing start reuses the Nexus task's request ID; every bypass-path start gets its own fresh, uncorrelated one. This change aligns Java with that. 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. It also covers the same-activity-ID restart pattern from sdk-python's regression test: starting a fresh run with an already-used activity ID within one invocation must not resolve to the stale, already-completed run. RootActivityClientInvokerTest covers the ambient-links-but-fresh-request-ID case, the outside-Nexus-context case, and two bypass-path starts for the same activity ID getting distinct request IDs, at the unit level.
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
2 participants
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
Only send request ID when in a backing nexus context when starting a SAA.
Why?
Sending request IDs for every SAA prevents starting multiple from the Nexus handler.
Checklist