Skip to content

Commit 2764436

Browse files
committed
Only set OnConflictOptions when there is a link or callback to attach
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.
1 parent 8fdbb33 commit 2764436

2 files changed

Lines changed: 33 additions & 11 deletions

File tree

‎temporal-sdk/src/main/java/io/temporal/internal/client/RootActivityClientInvoker.java‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -136,14 +136,19 @@ public StartActivityOutput startActivity(StartActivityInput input) {
136136
// completes the Nexus operation.
137137
protoLinks = nexusContext.getRequestLinks();
138138
request.addAllLinks(protoLinks);
139-
io.temporal.api.common.v1.OnConflictOptions.Builder onConflictOptions =
139+
}
140+
141+
boolean willAttachCompletionCallback =
142+
nexusOperationMetadata != null
143+
&& !Strings.isNullOrEmpty(nexusOperationMetadata.callbackUrl);
144+
if (nexusContext != null && (!protoLinks.isEmpty() || willAttachCompletionCallback)) {
145+
// The server rejects attach_request_id unless the request also carries at least one link
146+
// or completion callback to attach on conflict.
147+
request.setOnConflictOptions(
140148
io.temporal.api.common.v1.OnConflictOptions.newBuilder()
141149
.setAttachRequestId(true)
142-
.setAttachLinks(true);
143-
if (nexusOperationMetadata != null) {
144-
onConflictOptions.setAttachCompletionCallbacks(true);
145-
}
146-
request.setOnConflictOptions(onConflictOptions);
150+
.setAttachLinks(!protoLinks.isEmpty())
151+
.setAttachCompletionCallbacks(willAttachCompletionCallback));
147152
}
148153

149154
if (nexusOperationMetadata != null) {
@@ -159,7 +164,7 @@ public StartActivityOutput startActivity(StartActivityInput input) {
159164
"failed to generate activity operation token",
160165
e);
161166
}
162-
if (!Strings.isNullOrEmpty(nexusOperationMetadata.callbackUrl)) {
167+
if (willAttachCompletionCallback) {
163168
Callback cb =
164169
InternalUtils.buildNexusCallback(
165170
nexusOperationMetadata.callbackUrl,

‎temporal-sdk/src/test/java/io/temporal/internal/client/RootActivityClientInvokerTest.java‎

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -143,11 +143,30 @@ public void nexusMetadataWithEmptyCallbackUrlOmitsCompletionCallback() {
143143
Assert.assertEquals(0, request.getCompletionCallbacksCount());
144144
Assert.assertTrue(request.getOnConflictOptions().getAttachRequestId());
145145
Assert.assertTrue(request.getOnConflictOptions().getAttachLinks());
146-
Assert.assertTrue(request.getOnConflictOptions().getAttachCompletionCallbacks());
146+
Assert.assertFalse(request.getOnConflictOptions().getAttachCompletionCallbacks());
147147
Assert.assertNotNull(metadata.operationToken);
148148
Assert.assertEquals(Collections.singletonList(activityLink()), nexusContext.getResponseLinks());
149149
}
150150

151+
@Test
152+
public void metadataWithEmptyCallbackUrlAndNoLinksOmitsOnConflictOptions() {
153+
NexusOperationMetadata metadata =
154+
new NexusOperationMetadata(
155+
"nexus-request-id", "", Collections.singletonMap("Custom-Header", "value"));
156+
nexusContext.setNexusOperationMetadata(metadata);
157+
158+
invoker.startActivity(newStartActivityInput());
159+
160+
ArgumentCaptor<StartActivityExecutionRequest> captor =
161+
ArgumentCaptor.forClass(StartActivityExecutionRequest.class);
162+
verify(genericClient).startActivity(captor.capture());
163+
StartActivityExecutionRequest request = captor.getValue();
164+
Assert.assertEquals("nexus-request-id", request.getRequestId());
165+
Assert.assertEquals(0, request.getLinksCount());
166+
Assert.assertEquals(0, request.getCompletionCallbacksCount());
167+
Assert.assertFalse(request.hasOnConflictOptions());
168+
}
169+
151170
@Test
152171
public void nexusContextWithoutMetadataGetsAmbientLinksAndAmbientRequestIdButNoCallback() {
153172
Link link = workflowEventLink();
@@ -195,9 +214,7 @@ public void nexusContextWithoutAmbientStateStartsOrdinaryActivity() {
195214
Assert.assertFalse(request.getRequestId().isEmpty());
196215
Assert.assertEquals(0, request.getLinksCount());
197216
Assert.assertEquals(0, request.getCompletionCallbacksCount());
198-
Assert.assertTrue(request.getOnConflictOptions().getAttachRequestId());
199-
Assert.assertTrue(request.getOnConflictOptions().getAttachLinks());
200-
Assert.assertFalse(request.getOnConflictOptions().getAttachCompletionCallbacks());
217+
Assert.assertFalse(request.hasOnConflictOptions());
201218
Assert.assertEquals(Collections.singletonList(activityLink()), nexusContext.getResponseLinks());
202219
}
203220

0 commit comments

Comments
 (0)