Addressing PR comments

Author: Evanthx

.github/workflows/ci.yml

@@ -115,6 +115,7 @@ jobs:
               --dynamic-config-value frontend.activityAPIsEnabled=true \
               --dynamic-config-value activity.enableStandalone=true \
               --dynamic-config-value history.enableChasm=true \
+              --dynamic-config-value history.enableCHASMSignalBacklinks=true \
               --dynamic-config-value history.enableTransitionHistory=true &
           sleep 10s
 

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

@@ -9,6 +9,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
+import javax.annotation.Nonnull;
 
 public class InternalNexusOperationContext {
   private final String namespace;
@@ -18,22 +19,19 @@ public class InternalNexusOperationContext {
   private final WorkflowClient client;
   NexusOperationOutboundCallsInterceptor outboundCalls;
   // Links extracted from the inbound Nexus task. Stored once at the task-handler boundary so the
-  // workflow client (signal, signalWithStart) can attach them to outgoing requests via
-  // SignalWorkflowExecutionRequest.links.
+  // workflow client can attach them to the outgoing requests it issues (e.g. signal,
+  // signalWithStart) via the request's links field.
   private List<Link> nexusOperationLinks = Collections.emptyList();
-  // Backlinks returned by outbound RPCs the operation handler issues (currently
-  // SignalWorkflowExecutionResponse.link and SignalWithStartWorkflowExecutionResponse.signal_link).
-  // One entry per outbound RPC that returned a link. Drained by the task handler when building
-  // StartOperationResponse so each RPC the handler issued gets a corresponding link on the caller
-  // workflow's history event.
+  // Backlinks returned by outbound RPCs the operation handler issues (such as
+  // SignalWorkflowExecutionResponse.link or SignalWithStartWorkflowExecutionResponse.signal_link).
+  // One entry per outbound RPC that returned a link. Drained
+  // by the task handler when building StartOperationResponse so each RPC the handler issued gets a
+  // corresponding link on the caller workflow's history event.
   //
   // This context is only safe for use from the single thread that runs the operation handler (the
-  // Nexus task executor's thread). The mutators below assert this contract; a stray cross-thread
-  // call fails fast rather than silently corrupting the ArrayList.
+  // Nexus task executor's thread); the backing ArrayList is not synchronized. Handlers must not
+  // mutate it from other threads.
   private final List<Link> responseBacklinks = new ArrayList<>();
-  // Captured at construction (on the Nexus task executor's thread) and used to fail fast on any
-  // cross-thread mutation. See note on responseBacklinks.
-  private final Thread ownerThread;
 
   public InternalNexusOperationContext(
       String namespace,
@@ -46,7 +44,6 @@ public InternalNexusOperationContext(
     this.endpoint = endpoint;
     this.metricScope = metricScope;
     this.client = client;
-    this.ownerThread = Thread.currentThread();
   }
 
   public Scope getMetricsScope() {
@@ -88,15 +85,15 @@ public void setNexusOperationLinks(List<Link> links) {
     this.nexusOperationLinks = links == null ? Collections.emptyList() : links;
   }
 
-  /** Links from the inbound Nexus task; empty if none. Never null. */
-  public List<Link> getNexusOperationLinks() {
-    return nexusOperationLinks;
+  /** Links from the inbound Nexus task; empty if none. */
+  public @Nonnull List<Link> getNexusOperationLinks() {
+    return Collections.unmodifiableList(nexusOperationLinks);
   }
 
   /**
-   * Append a backlink returned by an outbound RPC the operation handler issued (signal or
-   * signalWithStart). The task handler drains the list when building the operation's
-   * StartOperationResponse.
+   * Append a backlink returned by an outbound RPC the operation handler issued (e.g. signal,
+   * signalWithStart, etc). The task handler drains the list when building the
+   * operation's StartOperationResponse.
    */
   public void addBacklink(Link link) {
     if (link != null) {
@@ -105,10 +102,13 @@ public void addBacklink(Link link) {
   }
 
   /**
-   * Backlinks from every outbound RPC the handler issued. Never null; may be empty. Returned as an
-   * unmodifiable view; callers must not attempt to mutate.
+   * Backlinks from every outbound RPC the handler issued. Returned as an unmodifiable view; callers
+   * must not attempt to mutate. Entries are accumulated while the operation handler runs (the call
+   * that flows through {@link
+   * io.temporal.common.interceptors.NexusOperationInboundCallsInterceptor#startOperation}) and are
+   * drained afterward by the task handler when building the StartOperationResponse.
    */
-  public List<Link> getBacklinks() {
+  public @Nonnull List<Link> getBacklinks() {
     return Collections.unmodifiableList(responseBacklinks);
   }
 

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

@@ -285,9 +285,9 @@ private StartOperationResponse handleStartOperation(
             .setCallbackUrl(task.getCallback())
             .setRequestId(task.getRequestId());
     task.getCallbackHeaderMap().forEach(operationStartDetails::putCallbackHeader);
-    // Stash the inbound links in common.v1.Link form on the operation context so that signal
-    // RPCs issued by the handler (e.g. SignalWithStartWorkflow on the callee) can attach them
-    // to SignalWorkflowExecutionRequest.links.
+    // Stash the inbound links in common.v1.Link form on the operation context so the RPCs the
+    // handler issues (e.g. signal, signalWithStart, etc) can attach them to their
+    // request's links field.
     List<io.temporal.api.common.v1.Link> inboundCommonLinks = new ArrayList<>();
     task.getLinksList()
         .forEach(
@@ -303,15 +303,15 @@ private StartOperationResponse handleStartOperation(
               }
               // LinkConverter only returns a WorkflowEvent-shaped common.v1.Link; nexus links of
               // other shapes (e.g. non-temporal URLs) come back null and are intentionally not
-              // forwarded onto SignalWorkflowExecutionRequest.links, which requires the
-              // WorkflowEvent variant. Log so a debugging session can see what was dropped.
+              // forwarded onto the RPCs the handler issues, which require the WorkflowEvent
+              // variant. Log so a debugging session can see what was dropped.
               io.temporal.api.common.v1.Link commonLink =
                   LinkConverter.nexusLinkToWorkflowEvent(link);
               if (commonLink != null) {
                 inboundCommonLinks.add(commonLink);
               } else {
                 log.warn(
-                    "Dropping inbound Nexus link from outbound signal propagation: type='{}',"
+                    "Dropping inbound Nexus link from outbound link propagation: type='{}',"
                         + " url='{}' (not a parseable temporal WorkflowEvent link)",
                     link.getType(),
                     link.getUrl());
@@ -328,9 +328,10 @@ private StartOperationResponse handleStartOperation(
       try {
         OperationStartResult<HandlerResultContent> result =
             startOperation(context, operationStartDetails.build(), input.build());
-        // If signal or signalWithStart RPCs the handler issued returned backlinks, propagate them
-        // to the caller so the caller workflow's history event links to each event on the callee.
-        // Same set of backlinks applies to both sync and async response variants.
+        // If any RPCs the handler issued (e.g. signal, signalWithStart, etc) returned
+        // backlinks, propagate them to the caller so the caller workflow's history event links to
+        // each event on the callee. Same set of backlinks applies to both sync and async response
+        // variants.
         List<io.temporal.api.nexus.v1.Link> backlinks = new ArrayList<>();
         for (io.temporal.api.common.v1.Link backlink :
             CurrentNexusOperationContext.get().getBacklinks()) {
@@ -388,8 +389,6 @@ private StartOperationResponse handleStartOperation(
             new RuntimeException("Unknown operation state: " + e.getState()));
       }
       startResponseBuilder.setFailure(dataConverter.exceptionToFailure(temporalFailure));
-    } catch (Throwable failure) {
-      convertKnownFailures(failure);
     }
     return startResponseBuilder.build();
   }