Deal with reserved __temporal prefix

Author: Sushisource

.editorconfig

@@ -47,6 +47,9 @@ dotnet_diagnostic.CA2237.severity = none
 # Warn on unused imports
 dotnet_diagnostic.IDE0005.severity = warning
 
+# Don't warn on using var
+dotnet_diagnostic.IDE0008.severity = none
+
 # Cannot use range operator on older versions
 dotnet_diagnostic.IDE0057.severity = none
 
@@ -186,4 +189,4 @@ dotnet_style_require_accessibility_modifiers = for_non_interface_members:suggest
 #Style - Pattern matching
 
 #prefer pattern matching instead of is expression with type casts
-csharp_style_pattern_matching_over_as_with_null_check = true:suggestion
+csharp_style_pattern_matching_over_as_with_null_check = true:suggestion

omnisharp.json

@@ -1,5 +1,8 @@
 {
   "FormattingOptions": {
     "EnableEditorConfigSupport": true
+  },
+  "RoslynExtensionsOptions": {
+    "enableAnalyzersSupport": true
   }
 }

src/Temporalio/Worker/WorkflowInstance.cs

@@ -107,7 +107,7 @@ public WorkflowInstance(WorkflowInstanceDetails details)
                         (WorkflowInboundInterceptor)rootInbound,
                         (v, impl) => impl.InterceptWorkflow(v));
                     ret.Init(new OutboundImpl(this));
-                    return ret;
+                    return new InboundReservedPrefixInterceptor(rootInbound, ret);
                 },
                 false);
             outbound = new(
@@ -939,7 +939,11 @@ private Task ApplyDoUpdateAsync(DoUpdate update)
             var updates = mutableUpdates.IsValueCreated ? mutableUpdates.Value : Definition.Updates;
             if (!updates.TryGetValue(update.Name, out var updateDefn))
             {
-                updateDefn = DynamicUpdate;
+                // Do not fall back onto dynamic update if using the reserved prefix
+                if (!update.Name.StartsWith(Workflow.ReservedHandlerPrefix))
+                {
+                    updateDefn = DynamicUpdate;
+                }
                 if (updateDefn == null)
                 {
                     var knownUpdates = updates.Keys.OrderBy(k => k);
@@ -1155,7 +1159,11 @@ private void ApplyQueryWorkflow(QueryWorkflow query)
                         var queries = mutableQueries.IsValueCreated ? mutableQueries.Value : Definition.Queries;
                         if (!queries.TryGetValue(query.QueryType, out queryDefn))
                         {
-                            queryDefn = DynamicQuery;
+                            // Do not fall back onto dynamic query if using the reserved prefix
+                            if (!query.QueryType.StartsWith(Workflow.ReservedHandlerPrefix))
+                            {
+                                queryDefn = DynamicQuery;
+                            }
                             if (queryDefn == null)
                             {
                                 var knownQueries = queries.Keys.OrderBy(k => k);
@@ -1267,7 +1275,11 @@ private void ApplySignalWorkflow(SignalWorkflow signal)
             var signals = mutableSignals.IsValueCreated ? mutableSignals.Value : Definition.Signals;
             if (!signals.TryGetValue(signal.SignalName, out var signalDefn))
             {
-                signalDefn = DynamicSignal;
+                // Do not fall back onto dynamic signal if using the reserved prefix
+                if (!signal.SignalName.StartsWith(Workflow.ReservedHandlerPrefix))
+                {
+                    signalDefn = DynamicSignal;
+                }
                 if (signalDefn == null)
                 {
                     // No definition found, buffer
@@ -2450,5 +2462,51 @@ public record Handler(
                 string? UpdateId,
                 HandlerUnfinishedPolicy UnfinishedPolicy);
         }
+
+        /// <summary>
+        /// This interceptor ensures that handlers using the reserved prefix bypass user
+        /// interceptors.
+        /// </summary>
+        private class InboundReservedPrefixInterceptor : WorkflowInboundInterceptor
+        {
+            private readonly InboundImpl root;
+            private readonly WorkflowInboundInterceptor complete;
+
+            public InboundReservedPrefixInterceptor(InboundImpl root, WorkflowInboundInterceptor complete)
+            {
+                this.root = root;
+                this.complete = complete;
+            }
+
+            public override void Init(WorkflowOutboundInterceptor outbound) => complete.Init(outbound);
+
+            public override Task<object?> ExecuteWorkflowAsync(ExecuteWorkflowInput input) =>
+                complete.ExecuteWorkflowAsync(input);
+
+            public override Task HandleSignalAsync(HandleSignalInput input) =>
+                input.Signal.StartsWith(Workflow.ReservedHandlerPrefix)
+                    ? root.HandleSignalAsync(input)
+                    : complete.HandleSignalAsync(input);
+
+            public override object? HandleQuery(HandleQueryInput input) =>
+                input.Query.StartsWith(Workflow.ReservedHandlerPrefix) ?
+                    root.HandleQuery(input) :
+                    complete.HandleQuery(input);
+
+            public override void ValidateUpdate(HandleUpdateInput input)
+            {
+                if (input.Update.StartsWith(Workflow.ReservedHandlerPrefix))
+                {
+                    root.ValidateUpdate(input);
+                    return;
+                }
+                complete.ValidateUpdate(input);
+            }
+
+            public override Task<object?> HandleUpdateAsync(HandleUpdateInput input) =>
+                input.Update.StartsWith(Workflow.ReservedHandlerPrefix) ?
+                    root.HandleUpdateAsync(input) :
+                    complete.HandleUpdateAsync(input);
+        }
     }
 }

src/Temporalio/Workflows/Workflow.cs

@@ -18,6 +18,21 @@ namespace Temporalio.Workflows
     /// </summary>
     public static class Workflow
     {
+        /// <summary>
+        /// Prefix for reserved handler names.
+        /// </summary>
+        internal const string ReservedHandlerPrefix = "__temporal";
+
+        /// <summary>
+        /// All known reserved query handler prefixes.
+        /// </summary>
+        internal static readonly string[] ReservedQueryHandlerPrefixes =
+        {
+            ReservedHandlerPrefix,
+            "__stack_trace",
+            "__enhanced_stack_trace",
+        };
+
         /// <summary>
         /// Gets a value indicating whether all update and signal handlers have finished executing.
         /// </summary>
@@ -1339,4 +1354,4 @@ public static class Unsafe
             public static bool IsReplaying => Context.IsReplaying;
         }
     }
-}
+}

src/Temporalio/Workflows/WorkflowDefinition.cs

@@ -427,6 +427,35 @@ public static WorkflowDefinition Create(
                 }
             }
 
+            // Verify that registered names to not use our reserved prefix
+            if (name != null && name.StartsWith(Workflow.ReservedHandlerPrefix))
+            {
+                errs.Add($"Workflow name {name} cannot start with {Workflow.ReservedHandlerPrefix}");
+            }
+            void CheckNamesForPrefix(IEnumerable<string> keys, string type)
+            {
+                foreach (var key in keys)
+                {
+                    if (key.StartsWith(Workflow.ReservedHandlerPrefix))
+                    {
+                        errs.Add($"{type} handler name {key} cannot start with {Workflow.ReservedHandlerPrefix}");
+                    }
+                    else if (type == "Query")
+                    {
+                        foreach (var reservedQ in Workflow.ReservedQueryHandlerPrefixes)
+                        {
+                            if (key.StartsWith(reservedQ))
+                            {
+                                errs.Add($"{type} handler name {key} cannot start with {reservedQ}");
+                            }
+                        }
+                    }
+                }
+            }
+            CheckNamesForPrefix(signals.Keys, "Signal");
+            CheckNamesForPrefix(queries.Keys, "Query");
+            CheckNamesForPrefix(updates.Keys, "Update");
+
             // If there are any errors, throw
             if (errs.Count > 0)
             {
@@ -525,4 +554,4 @@ private static bool IsDefinedOnBase<T>(MethodInfo method)
             }
         }
     }
-}
+}

tests/Temporalio.Tests/Worker/WorkflowWorkerTests.cs

@@ -2997,6 +2997,17 @@ await ExecuteWorkerAsync<DynamicWorkflow>(
                 await handle.SignalAsync(wf => wf.SomeSignalAsync("signal arg"));
                 Assert.Equal("done", await handle.QueryAsync(wf => wf.SomeQuery("query arg")));
                 Assert.Equal("done", await handle.ExecuteUpdateAsync(wf => wf.SomeUpdateAsync("update arg")));
+
+                // Verify that invocations using the reserved prefix are not forwarded to dynamic
+                // handlers
+                await handle.SignalAsync($"{Workflow.ReservedHandlerPrefix}_whatever", new[] { "signal arg 1" });
+                var queryExc2 = await Assert.ThrowsAsync<WorkflowQueryFailedException>(
+                    () => handle.QueryAsync<string>($"{Workflow.ReservedHandlerPrefix}_whatever", new[] { "query arg 1" }));
+                Assert.Contains("not found", queryExc2.Message);
+                var updateExc = await Assert.ThrowsAsync<WorkflowUpdateFailedException>(
+                    () => handle.ExecuteUpdateAsync<string>($"{Workflow.ReservedHandlerPrefix}_whatever", new[] { "update arg 1" }));
+                Assert.Contains("not found", updateExc.InnerException?.Message);
+
                 // Event list must be collected before the WF finishes, since when it finishes it
                 // will be evicted from the cache and the first SomeQuery event will not exist upon
                 // replay.

tests/Temporalio.Tests/Workflows/WorkflowDefinitionTests.cs

@@ -234,6 +234,17 @@ public void Create_BadUpdates_Throws()
         AssertBad<Bad.WfUpdate>("ValidateGenericUpdate[TLocal](TLocal) with WorkflowUpdateValidator contains generic parameters");
     }
 
+    [Fact]
+    public void Reserved_Handler_Prefixes_Throws()
+    {
+        AssertBad<Bad.BadWfName>("Workflow name __temporal_why_would_you_do_this cannot start with __temporal");
+        AssertBad<Bad.BadHandlerNames>("Signal handler name __temporal_dumb_signal cannot start with __temporal");
+        AssertBad<Bad.BadHandlerNames>("Query handler name __temporal_dumb_query cannot start with __temporal");
+        AssertBad<Bad.BadHandlerNames>("Query handler name __stack_trace cannot start with __stack_trace");
+        AssertBad<Bad.BadHandlerNames>("Query handler name __enhanced_stack_trace cannot start with __enhanced_stack_trace");
+        AssertBad<Bad.BadHandlerNames>("Update handler name __temporal_dumb_update cannot start with __temporal");
+    }
+
     private static void AssertBad<T>(string errContains)
     {
         var err = Assert.ThrowsAny<Exception>(() => WorkflowDefinition.Create(typeof(T)));
@@ -524,6 +535,35 @@ protected void ValidateUpdate5()
             {
             }
         }
+
+        [Workflow("__temporal_why_would_you_do_this")]
+        public class BadWfName
+        {
+            [WorkflowRun]
+            public Task RunAsync() => Task.CompletedTask;
+        }
+
+        [Workflow]
+        public class BadHandlerNames
+        {
+            [WorkflowRun]
+            public Task RunAsync() => Task.CompletedTask;
+
+            [WorkflowSignal("__temporal_dumb_signal")]
+            public Task SignalAsync() => Task.CompletedTask;
+
+            [WorkflowQuery("__temporal_dumb_query")]
+            public string Query() => string.Empty;
+
+            [WorkflowQuery("__stack_trace")]
+            public string StackTraceImpostor() => string.Empty;
+
+            [WorkflowQuery("__enhanced_stack_trace")]
+            public string EnhancedStackTraceImpostor() => string.Empty;
+
+            [WorkflowUpdate("__temporal_dumb_update")]
+            public Task UpdateAsync() => Task.CompletedTask;
+        }
     }
 
     public static class Good
@@ -583,4 +623,4 @@ public Wf2(string param1, int param2 = 5)
             public override Task SomeSignalAsync() => Task.CompletedTask;
         }
     }
-}
+}