From 771c8516ff522cd1af62f61bf702490f41300ebc Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 21 Apr 2026 10:57:07 +0000 Subject: [PATCH 1/3] Validate step types --- .../Process.Core/ProcessStepBuilder.cs | 4 + .../DaprStepInfoTests.cs | 87 +++++++++++++++++++ .../TypeInfoTests.cs | 74 ++++++++++++++++ .../Process.Runtime.Dapr/Actors/StepActor.cs | 10 +++ .../Process.Runtime.Dapr/DaprStepInfo.cs | 5 ++ .../Serialization/TypeInfo.cs | 7 +- .../Core/ProcessBuilderTests.cs | 31 +++++++ 7 files changed, 217 insertions(+), 1 deletion(-) create mode 100644 dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs create mode 100644 dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/TypeInfoTests.cs diff --git a/dotnet/src/Experimental/Process.Core/ProcessStepBuilder.cs b/dotnet/src/Experimental/Process.Core/ProcessStepBuilder.cs index d0a63da1a274..9ead1b39167c 100644 --- a/dotnet/src/Experimental/Process.Core/ProcessStepBuilder.cs +++ b/dotnet/src/Experimental/Process.Core/ProcessStepBuilder.cs @@ -278,6 +278,10 @@ internal ProcessStepBuilderTyped(Type stepType, string id, ProcessBuilder? proce : base(id, processBuilder) { Verify.NotNull(stepType); + if (!typeof(KernelProcessStep).IsAssignableFrom(stepType)) + { + throw new ArgumentException($"Type '{stepType.FullName}' must be a subclass of KernelProcessStep.", nameof(stepType)); + } this._stepType = stepType; this.FunctionsDict = this.GetFunctionMetadataMap(); diff --git a/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs b/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs new file mode 100644 index 000000000000..51d86062a479 --- /dev/null +++ b/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs @@ -0,0 +1,87 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections.Generic; +using Microsoft.SemanticKernel; +using Xunit; + +namespace SemanticKernel.Process.Dapr.Runtime.UnitTests; + +/// +/// Unit tests for the class. +/// +public class DaprStepInfoTests +{ + /// + /// Tests that ToKernelProcessStepInfo throws when InnerStepDotnetType is not a KernelProcessStep subclass. + /// + [Fact] + public void ToKernelProcessStepInfoThrowsForInvalidStepType() + { + // Arrange + var stepInfo = new DaprStepInfo + { + InnerStepDotnetType = typeof(string).AssemblyQualifiedName!, + State = new KernelProcessStepState("TestStep", version: "v1"), + Edges = new Dictionary>() + }; + + // Act & Assert + var ex = Assert.Throws(() => stepInfo.ToKernelProcessStepInfo()); + Assert.Contains("is not a valid KernelProcessStep type", ex.Message); + } + + /// + /// Tests that ToKernelProcessStepInfo throws when InnerStepDotnetType cannot be resolved. + /// + [Fact] + public void ToKernelProcessStepInfoThrowsForUnresolvableType() + { + // Arrange + var stepInfo = new DaprStepInfo + { + InnerStepDotnetType = "NonExistent.Type, NonExistent.Assembly", + State = new KernelProcessStepState("TestStep", version: "v1"), + Edges = new Dictionary>() + }; + + // Act & Assert + Assert.Throws(() => stepInfo.ToKernelProcessStepInfo()); + } + + /// + /// Tests that ToKernelProcessStepInfo succeeds for a valid KernelProcessStep subclass. + /// + [Fact] + public void ToKernelProcessStepInfoSucceedsForValidStepType() + { + // Arrange + var stepInfo = new DaprStepInfo + { + InnerStepDotnetType = typeof(ValidTestStep).AssemblyQualifiedName!, + State = new KernelProcessStepState("TestStep", version: "v1"), + Edges = new Dictionary>() + }; + + // Act + var result = stepInfo.ToKernelProcessStepInfo(); + + // Assert + Assert.NotNull(result); + Assert.Equal(typeof(ValidTestStep), result.InnerStepType); + } + + /// + /// A valid test step for type validation testing. + /// + public sealed class ValidTestStep : KernelProcessStep + { + /// + /// A test function. + /// + [KernelFunction] + public void TestFunction() + { + } + } +} diff --git a/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/TypeInfoTests.cs b/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/TypeInfoTests.cs new file mode 100644 index 000000000000..24e532b412f7 --- /dev/null +++ b/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/TypeInfoTests.cs @@ -0,0 +1,74 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System.Text.Json; +using Microsoft.SemanticKernel; +using Microsoft.SemanticKernel.Process.Serialization; +using Xunit; + +namespace SemanticKernel.Process.Dapr.Runtime.UnitTests; + +/// +/// Unit tests for the class. +/// +public class TypeInfoTests +{ + /// + /// Tests that ConvertValue deserializes a JsonElement to the correct type. + /// + [Fact] + public void ConvertValueDeserializesJsonElement() + { + // Arrange + var json = JsonSerializer.SerializeToElement(42); + var typeName = typeof(int).AssemblyQualifiedName; + + // Act + var result = TypeInfo.ConvertValue(typeName, json); + + // Assert + Assert.Equal(42, result); + } + + /// + /// Tests that ConvertValue throws when the type name cannot be resolved. + /// + [Fact] + public void ConvertValueThrowsForUnresolvableType() + { + // Arrange + var json = JsonSerializer.SerializeToElement(42); + + // Act & Assert + Assert.Throws(() => + TypeInfo.ConvertValue("NonExistent.Type, NonExistent.Assembly", json)); + } + + /// + /// Tests that ConvertValue returns non-JsonElement values unchanged. + /// + [Fact] + public void ConvertValueReturnsNonJsonElementUnchanged() + { + // Arrange + var value = "plain string"; + + // Act + var result = TypeInfo.ConvertValue(typeof(string).AssemblyQualifiedName, value); + + // Assert + Assert.Equal("plain string", result); + } + + /// + /// Tests that ConvertValue returns null when value is null. + /// + [Fact] + public void ConvertValueReturnsNullWhenValueIsNull() + { + // Act + var result = TypeInfo.ConvertValue(typeof(string).AssemblyQualifiedName, null); + + // Assert + Assert.Null(result); + } +} diff --git a/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs b/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs index 27bda37176fb..540f3449e159 100644 --- a/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs +++ b/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs @@ -102,6 +102,11 @@ private void InitializeStep(DaprStepInfo stepInfo, string? parentProcessId, stri throw new KernelException($"Could not load the inner step type '{stepInfo.InnerStepDotnetType}'.").Log(this._logger); } + if (!typeof(KernelProcessStep).IsAssignableFrom(this._innerStepType)) + { + throw new KernelException($"Type '{stepInfo.InnerStepDotnetType}' is not a valid KernelProcessStep type.").Log(this._logger); + } + this.ParentProcessId = parentProcessId; this._stepInfo = stepInfo; this._stepState = this._stepInfo.State; @@ -373,6 +378,11 @@ protected virtual async ValueTask ActivateStepAsync() if (stepStateType.HasValue) { stateType = Type.GetType(stepStateType.Value); + if (stateType is not null && !typeof(KernelProcessStepState).IsAssignableFrom(stateType)) + { + throw new KernelException($"Type '{stepStateType.Value}' is not a valid KernelProcessStepState type.").Log(this._logger); + } + var stateObjectJson = await this.StateManager.GetStateAsync(ActorStateKeys.StepStateJson).ConfigureAwait(false); stateObject = JsonSerializer.Deserialize(stateObjectJson, stateType!) as KernelProcessStepState; } diff --git a/dotnet/src/Experimental/Process.Runtime.Dapr/DaprStepInfo.cs b/dotnet/src/Experimental/Process.Runtime.Dapr/DaprStepInfo.cs index 15b72d76744d..93a30df4008a 100644 --- a/dotnet/src/Experimental/Process.Runtime.Dapr/DaprStepInfo.cs +++ b/dotnet/src/Experimental/Process.Runtime.Dapr/DaprStepInfo.cs @@ -50,6 +50,11 @@ public KernelProcessStepInfo ToKernelProcessStepInfo() throw new KernelException($"Unable to create inner step type from assembly qualified name `{this.InnerStepDotnetType}`"); } + if (!typeof(KernelProcessStep).IsAssignableFrom(innerStepType)) + { + throw new KernelException($"Type '{this.InnerStepDotnetType}' is not a valid KernelProcessStep type."); + } + return new KernelProcessStepInfo(innerStepType, this.State, this.Edges); } diff --git a/dotnet/src/Experimental/Process.Runtime.Dapr/Serialization/TypeInfo.cs b/dotnet/src/Experimental/Process.Runtime.Dapr/Serialization/TypeInfo.cs index ad64a1e1a53c..9cbca889783b 100644 --- a/dotnet/src/Experimental/Process.Runtime.Dapr/Serialization/TypeInfo.cs +++ b/dotnet/src/Experimental/Process.Runtime.Dapr/Serialization/TypeInfo.cs @@ -39,6 +39,11 @@ internal static class TypeInfo } Type? valueType = Type.GetType(assemblyQualifiedTypeName); - return ((JsonElement)value).Deserialize(valueType!); + if (valueType is null) + { + throw new KernelException($"Could not load type '{assemblyQualifiedTypeName}'."); + } + + return ((JsonElement)value).Deserialize(valueType); } } diff --git a/dotnet/src/Experimental/Process.UnitTests/Core/ProcessBuilderTests.cs b/dotnet/src/Experimental/Process.UnitTests/Core/ProcessBuilderTests.cs index 26c2fc9b0e7e..8d8ea820513b 100644 --- a/dotnet/src/Experimental/Process.UnitTests/Core/ProcessBuilderTests.cs +++ b/dotnet/src/Experimental/Process.UnitTests/Core/ProcessBuilderTests.cs @@ -167,6 +167,37 @@ public void OnFunctionErrorCreatesEdgeBuilder() Assert.EndsWith("Global.OnError", edgeBuilder.EventData.EventId); } + /// + /// Tests that AddStepFromType(Type) throws ArgumentException for non-KernelProcessStep types. + /// + [Fact] + public void AddStepFromTypeWithInvalidTypeThrowsArgumentException() + { + // Arrange + var processBuilder = new ProcessBuilder(ProcessName); + + // Act & Assert + var ex = Assert.Throws(() => processBuilder.AddStepFromType(typeof(string))); + Assert.Contains("must be a subclass of KernelProcessStep", ex.Message); + } + + /// + /// Tests that AddStepFromType(Type) succeeds for valid KernelProcessStep types. + /// + [Fact] + public void AddStepFromTypeWithValidTypeAddsStep() + { + // Arrange + var processBuilder = new ProcessBuilder(ProcessName); + + // Act + var stepBuilder = processBuilder.AddStepFromType(typeof(TestStep), StepName); + + // Assert + Assert.Single(processBuilder.Steps); + Assert.Equal(StepName, stepBuilder.Name); + } + /// /// A class that represents a step for testing. /// From a5e369d20a8d2884c081e3f2a21b7a54b6a8086d Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:01:54 +0000 Subject: [PATCH 2/3] Fix format issue --- .../Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs b/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs index 51d86062a479..60b81ab16428 100644 --- a/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs +++ b/dotnet/src/Experimental/Process.Runtime.Dapr.UnitTests/DaprStepInfoTests.cs @@ -1,6 +1,5 @@ // Copyright (c) Microsoft. All rights reserved. -using System; using System.Collections.Generic; using Microsoft.SemanticKernel; using Xunit; From 0694475e2ba68c135597902331519389da1b8368 Mon Sep 17 00:00:00 2001 From: westey <164392973+westey-m@users.noreply.github.com> Date: Tue, 21 Apr 2026 11:21:25 +0000 Subject: [PATCH 3/3] Address PR comment --- .../Process.Runtime.Dapr/Actors/StepActor.cs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs b/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs index 540f3449e159..ff8545efb73b 100644 --- a/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs +++ b/dotnet/src/Experimental/Process.Runtime.Dapr/Actors/StepActor.cs @@ -378,13 +378,18 @@ protected virtual async ValueTask ActivateStepAsync() if (stepStateType.HasValue) { stateType = Type.GetType(stepStateType.Value); - if (stateType is not null && !typeof(KernelProcessStepState).IsAssignableFrom(stateType)) + if (stateType is null) + { + throw new KernelException($"Type '{stepStateType.Value}' could not be resolved to a valid KernelProcessStepState type.").Log(this._logger); + } + + if (!typeof(KernelProcessStepState).IsAssignableFrom(stateType)) { throw new KernelException($"Type '{stepStateType.Value}' is not a valid KernelProcessStepState type.").Log(this._logger); } var stateObjectJson = await this.StateManager.GetStateAsync(ActorStateKeys.StepStateJson).ConfigureAwait(false); - stateObject = JsonSerializer.Deserialize(stateObjectJson, stateType!) as KernelProcessStepState; + stateObject = JsonSerializer.Deserialize(stateObjectJson, stateType) as KernelProcessStepState; } else {