Respond to PR comments

Author: Quinn-With-Two-Ns

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

@@ -3,6 +3,7 @@
 import static io.temporal.internal.common.RetryOptionsUtils.toRetryPolicy;
 import static io.temporal.internal.common.WorkflowExecutionUtils.makeUserMetaData;
 
+import com.google.common.base.Strings;
 import com.google.common.collect.Iterators;
 import io.grpc.Deadline;
 import io.grpc.Status;
@@ -132,13 +133,15 @@ public StartActivityOutput startActivity(StartActivityInput input) {
             "failed to generate activity operation token",
             e);
       }
-      Callback cb =
-          InternalUtils.buildNexusCallback(
-              nexusOperationMetadata.callbackUrl,
-              nexusOperationMetadata.callbackHeaders,
-              nexusOperationMetadata.operationToken,
-              protoLinks);
-      request.addCompletionCallbacks(cb);
+      if (!Strings.isNullOrEmpty(nexusOperationMetadata.callbackUrl)) {
+        Callback cb =
+            InternalUtils.buildNexusCallback(
+                nexusOperationMetadata.callbackUrl,
+                nexusOperationMetadata.callbackHeaders,
+                nexusOperationMetadata.operationToken,
+                protoLinks);
+        request.addCompletionCallbacks(cb);
+      }
     }
 
     StartActivityExecutionResponse response;

temporal-sdk/src/main/java/io/temporal/internal/nexus/OperationTokenType.java

@@ -6,8 +6,8 @@
 public enum OperationTokenType {
   UNKNOWN(0),
   WORKFLOW_RUN(1),
-  // Values 2 and 3 are reserved for future token types (e.g. update-workflow, get-workflow-result).
-  ACTIVITY_EXECUTION(4);
+  ACTIVITY_EXECUTION(2),
+  UPDATE_WORKFLOW(3);
 
   private final int value;
 

temporal-sdk/src/main/java/io/temporal/nexus/TemporalNexusClientImpl.java

@@ -516,16 +516,17 @@ private <R> TemporalOperationResult<R> startActivityImpl(
                             .build());
                 ActivityClientCallsInterceptor.StartActivityOutput out =
                     ((ActivityClientInternal) activityClient).getInvoker().startActivity(input);
-                // The invoker generated and injected the runId-free token into the callback
-                // headers before the start RPC fired. The header token cannot include a run ID
-                // because the run ID isn't known until after the start RPC returns. The operation
-                // token returned to the Nexus caller can — and should — include it, so it's
-                // regenerated here from the same activity ID + the run ID the start RPC produced.
+                // The invoker generated the runId-free token before the start RPC and injected it
+                // into the callback headers when a callback URL was supplied. That token cannot
+                // include a run ID because the run ID isn't known until after the start RPC
+                // returns. The operation token returned to the Nexus caller can — and should —
+                // include it, so it's regenerated here from the same activity ID + the run ID the
+                // start RPC produced.
                 String headerToken = nexusOperationMetadata.operationToken;
                 if (headerToken == null) {
                   throw new HandlerException(
                       HandlerException.ErrorType.INTERNAL,
-                      "invoker did not return a Nexus operation token for activity start with callback",
+                      "invoker did not generate a Nexus operation token for activity start",
                       new IllegalStateException(
                           "operationToken is null on NexusOperationMetadata after activity start"));
                 }

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

@@ -103,6 +103,28 @@ public void nexusMetadataAddsCallbackLinksAndRequestId() {
     Assert.assertEquals(Collections.singletonList(activityLink()), nexusContext.getResponseLinks());
   }
 
+  @Test
+  public void nexusMetadataWithEmptyCallbackUrlOmitsCompletionCallback() {
+    NexusOperationMetadata metadata =
+        new NexusOperationMetadata(
+            "nexus-request-id", "", Collections.singletonMap("Custom-Header", "value"));
+    nexusContext.setNexusOperationMetadata(metadata);
+    Link link = workflowEventLink();
+    nexusContext.setRequestLinks(Collections.singletonList(link));
+
+    invoker.startActivity(newStartActivityInput());
+
+    ArgumentCaptor<StartActivityExecutionRequest> captor =
+        ArgumentCaptor.forClass(StartActivityExecutionRequest.class);
+    verify(genericClient).startActivity(captor.capture());
+    StartActivityExecutionRequest request = captor.getValue();
+    Assert.assertEquals("nexus-request-id", request.getRequestId());
+    Assert.assertEquals(Collections.singletonList(link), request.getLinksList());
+    Assert.assertEquals(0, request.getCompletionCallbacksCount());
+    Assert.assertNotNull(metadata.operationToken);
+    Assert.assertEquals(Collections.singletonList(activityLink()), nexusContext.getResponseLinks());
+  }
+
   @Test
   public void nexusContextWithoutMetadataStartsOrdinaryActivity() {
     nexusContext.setRequestLinks(Collections.singletonList(workflowEventLink()));

…internal/nexus/WorkflowRunTokenTest.java …l/internal/nexus/OperationTokenTest.javatemporal-sdk/src/test/java/io/temporal/internal/nexus/WorkflowRunTokenTest.java renamed to temporal-sdk/src/test/java/io/temporal/internal/nexus/OperationTokenTest.java temporal-sdk/src/test/java/io/temporal/internal/nexus/WorkflowRunTokenTest.java renamed to temporal-sdk/src/test/java/io/temporal/internal/nexus/OperationTokenTest.java

@@ -8,13 +8,21 @@
 import org.junit.Assert;
 import org.junit.Test;
 
-public class WorkflowRunTokenTest {
+public class OperationTokenTest {
   private static final ObjectWriter ow =
       new ObjectMapper().registerModule(new Jdk8Module()).writer();
   private static final ObjectReader or =
       new ObjectMapper().registerModule(new Jdk8Module()).reader();
   private static final Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding();
 
+  @Test
+  public void operationTokenTypeValues() {
+    Assert.assertEquals(0, OperationTokenType.UNKNOWN.toValue());
+    Assert.assertEquals(1, OperationTokenType.WORKFLOW_RUN.toValue());
+    Assert.assertEquals(2, OperationTokenType.ACTIVITY_EXECUTION.toValue());
+    Assert.assertEquals(3, OperationTokenType.UPDATE_WORKFLOW.toValue());
+  }
+
   @Test
   public void serializeWorkflowRunToken() throws JsonProcessingException {
     OperationToken token =
@@ -140,7 +148,7 @@ public void workflowRunTokenBytesByteIdenticalSnapshot() throws JsonProcessingEx
 
   @Test
   public void malformedActivityTokenRejected() {
-    String malformed = "{\"t\":4,\"ns\":\"ns\",\"aid\":\"\"}";
+    String malformed = "{\"t\":2,\"ns\":\"ns\",\"aid\":\"\"}";
     IllegalArgumentException ex =
         Assert.assertThrows(
             IllegalArgumentException.class,

temporal-sdk/src/test/java/io/temporal/workflow/nexus/AsyncActivityOperationTest.java

@@ -45,7 +45,7 @@ protected SDKTestWorkflowRule getTestWorkflowRule() {
   public void testActivityOperationEndToEnd() {
     // The in-process test-server does not implement the StartActivityExecution RPC; the
     // standalone-activity Nexus path requires a real server. Unit-only token assertions stay
-    // active in testActivityOperationTokenRoundTrip below.
+    // active in OperationTokenTest.
     assumeTrue(SDKTestWorkflowRule.useExternalService);
     // Combined (a) caller workflow receives "hello " + input
     //          (b) opToken loads as ACTIVITY_EXECUTION with aid="act-" + requestId
@@ -55,25 +55,13 @@ public void testActivityOperationEndToEnd() {
     Assert.assertEquals("hello world", result);
   }
 
-  @Test
-  public void testActivityOperationTokenRoundTrip() throws Exception {
-    // (c) generateActivityExecutionOperationToken then loadOperationToken round-trips equal
-    //     aid + ns.
-    String token = OperationTokenUtil.generateActivityExecutionOperationToken("act-123", "test-ns");
-    OperationToken loaded = OperationTokenUtil.loadOperationToken(token);
-    Assert.assertEquals(OperationTokenType.ACTIVITY_EXECUTION, loaded.getType());
-    Assert.assertEquals("act-123", loaded.getActivityId());
-    Assert.assertEquals("test-ns", loaded.getNamespace());
-    Assert.assertNull(loaded.getWorkflowId());
-  }
-
   @Test
   public void testDoubleStartActivityThrows() {
     // The first start RPC requires StartActivityExecution which is not implemented by the
     // in-process test server; gate this on a real server so the guard at the second call can be
     // observed.
     assumeTrue(SDKTestWorkflowRule.useExternalService);
-    // (d) Calling startActivity twice in one handler invocation throws
+    // (c) Calling startActivity twice in one handler invocation throws
     //     HandlerException(BAD_REQUEST).
     TestDoubleStartWorkflow workflowStub =
         testWorkflowRule.newWorkflowStubTimeoutOptions(TestDoubleStartWorkflow.class);