From b1254a30dca7cf792879121ed16cd226935efcd3 Mon Sep 17 00:00:00 2001 From: AbigailDeng <108705114+AbigailDeng@users.noreply.github.com> Date: Tue, 27 Oct 2026 12:11:57 +0800 Subject: [PATCH] Validate workflow YAML structure before normalization --- docs/canon/workflow-runtime.md | 30 +- .../Studio/Services/WorkflowEditorService.cs | 18 +- .../YamlWorkflowDocumentService.cs | 9 +- .../WorkflowParser.Structure.Presentation.cs | 158 ++++++++ .../Primitives/WorkflowParser.Structure.cs | 282 ++++++++++++++ .../Primitives/WorkflowParser.cs | 37 +- .../WorkflowYamlStructureException.cs | 18 + .../Primitives/WorkflowYamlValidatorImpl.cs | 5 + .../EditorControllerSerializationTests.cs | 38 +- .../WorkflowEditorServiceStructureTests.cs | 170 ++++++++ .../Primitives/WorkflowParserCoverageTests.cs | 1 - ...orkflowParserPresentationStructureTests.cs | 227 +++++++++++ .../WorkflowParserStructureTests.cs | 368 ++++++++++++++++++ 13 files changed, 1325 insertions(+), 36 deletions(-) create mode 100644 src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.Presentation.cs create mode 100644 src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.cs create mode 100644 src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlStructureException.cs create mode 100644 test/Aevatar.Studio.Tests/WorkflowEditorServiceStructureTests.cs create mode 100644 test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserPresentationStructureTests.cs create mode 100644 test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserStructureTests.cs diff --git a/docs/canon/workflow-runtime.md b/docs/canon/workflow-runtime.md index dd0c571b97..0347d3beb5 100644 --- a/docs/canon/workflow-runtime.md +++ b/docs/canon/workflow-runtime.md @@ -72,8 +72,8 @@ owner: eanzhao ``` BindWorkflowDefinition(yaml) - -> WorkflowParser.Parse (YAML -> WorkflowDefinition) - -> WorkflowValidator.Validate (结构校验) + -> WorkflowParser.Parse (资源限制 -> YAML 语法 -> 原始 AST 结构 -> WorkflowDefinition 映射) + -> WorkflowValidator.Validate (定义语义与引用校验) -> BindWorkflowRunDefinition(yaml/run binding + capability admission plan) -> InstallCognitiveModules on WorkflowRunGAgent: IWorkflowModuleDependencyExpander[]: 推导模块名集合 @@ -82,6 +82,32 @@ BindWorkflowDefinition(yaml) WorkflowExecutionBridgeModule: 接入 Foundation 事件管线 ``` +### YAML 结构校验边界 + +Workflow YAML 使用同一条解析主干：先限制输入资源，再解析 YAML 语法，然后校验原始 AST 的结构，通过后才映射为 `WorkflowDefinition`，最后由 `WorkflowValidator` 校验定义语义。结构校验必须发生在默认值补齐、别名归一化与对象映射之前，防止错误节点在转换时变成空集合、缺失字段或默认值，导致后续语义校验失去原始错误证据。 + +```mermaid +%%{init: {"maxTextSize": 100000, "flowchart": {"useMaxWidth": false, "nodeSpacing": 10, "rankSpacing": 50}, "themeVariables": {"fontSize": "10px"}}}%% +flowchart LR + Y["Workflow YAML"] --> R["资源限制"] + R --> P["YAML 语法解析"] + P --> S["原始 AST 结构校验"] + S --> M["WorkflowDefinition 映射"] + M --> V["定义语义与引用校验"] + V --> A["Definition admission 与 actor 命令"] + S -->|"结构错误"| E["yaml_structure + 字段路径"] +``` + +`WorkflowParser.Parse` 是运行时的权威入口；`WorkflowParser.ValidateStructure(YamlStream)` 让 Studio 的 AST 解析复用同一套结构规则。接收 AST 的方法不替代输入资源限制，调用方仍须先执行 `WorkflowYamlResourceGuard` 与 YAML 语法解析。Studio 在构造编辑文档、归一化或序列化之前检查原始 AST，不能先删去非法节点再用转换结果证明原文有效。所有使用 canonical parser 的保存、preview、绑定、发布和执行路径都经过该结构边界；是否允许保存未通过语义校验的编辑草稿，仍由对应资源入口明确决定。 + +结构规则从 parser 的 `Raw` 强类型 YAML 合约派生静态字段名与容器形状，覆盖根 mapping、角色和步骤列表、嵌套 children、configuration、retry、capability 与 response projection 等对象。未知静态字段以及预期 scalar、mapping 或 sequence 却给出其他节点类型的输入，在映射前拒绝。`branches` 是有明确格式的控制流字段：只接受分支 mapping 或分支条目列表。mapping 格式的分支目标支持非空 scalar，或只声明一个目标别名的单层 mapping；列表中的每一项必须是 mapping，并分别给出一个非空 scalar 条件与目标。递归目标 mapping 不属于该格式，禁止通过跳过不完整条目或静默覆盖重复条件来修复输入。角色、步骤和目标是否存在、原语是否合法、控制流规则是否成立，继续由映射后的语义 validator 负责。 + +权限与交互字段即使在 parser 中采用 `object` 接收，也不作为任意结构开放：`allowed_tools`、`tool_sets` 只支持 scalar 或 scalar 列表，包括既有的 JSON array string 格式；列表中禁止 object 与 null 元素。`interaction_spec`、`interaction_template_spec` 及 inline presentation 的 actions、fields、cards 依据既有 Protobuf descriptor 校验对象、列表和字段结构，保留 mapper 已支持的字段别名与 JSON object/array string 格式，拒绝未知交互字段及非法列表元素。相同语义字段的别名不得重复声明，交互 bool 字段只接受 `true` 或 `false`；`template_variable` 则明确保留既有格式：复杂变量值转换成 JSON 字符串后写入 typed string map。step-level 字段与 `parameters` 中对应的保留字段复用同一组规则。 + +可选字段的显式 YAML `null` 与省略保持同一语义，例如 `configuration: null` 或 `children: null`；带引号的 `"null"` 或显式 `!!str null` 仍是字符串。可选字段允许 null 不代表 typed list 可以包含空元素：`roles`、`steps`、`children`、response projection operation 与交互对象等列表一旦包含 null 元素，必须报告元素路径并拒绝。`parameters` 是明确的原语参数扩展边界，除上述有正式语义的保留字段外，合法的 mapping、sequence、scalar 与 null 值继续保留，不以禁止复杂参数值代替结构校验；这些值的业务含义仍由所属原语合约与语义校验解释。 + +结构错误使用 `WorkflowYamlStructureException`：节点形状错误携带 `yaml_structure`，未知字段携带 `unknown_field`，两者均保留原始字段路径，例如 `/steps/0/children/1`。路径采用 JSON Pointer 转义，键中的 `~` 与 `/` 分别表示为 `~0` 与 `~1`。Studio 将错误码与路径保留到 `ValidationFinding`，让作者能区分 YAML 语法错误、原始节点结构错误和 definition 语义错误。结构失败不得回退为忽略字段、空分支、默认类型或成功的归一化结果。 + ### Normalized execution state rollout `workflow.run` 的 state schema v1 把执行值的引用身份从 legacy string map 中分离出来；schema v2 在此基础上增加 author-directed value lifecycle、digest replay evidence 与 release tombstone。`WorkflowExecutionKernelState.normalized_values` 是否存在是完整的 legacy/normalized 判别；业务 state 不再复制 runtime schema version，版本轴只来自 `RuntimeActorIdentity.state_schema_version`。 diff --git a/src/Aevatar.Studio.Application/Studio/Services/WorkflowEditorService.cs b/src/Aevatar.Studio.Application/Studio/Services/WorkflowEditorService.cs index 318b7c7ce6..5d1b132b69 100644 --- a/src/Aevatar.Studio.Application/Studio/Services/WorkflowEditorService.cs +++ b/src/Aevatar.Studio.Application/Studio/Services/WorkflowEditorService.cs @@ -4,6 +4,7 @@ using Aevatar.Studio.Domain.Studio.Services; using RuntimeWorkflowPrimitiveCatalog = Aevatar.Workflow.Core.Primitives.WorkflowPrimitiveCatalog; using RuntimeWorkflowParser = Aevatar.Workflow.Core.Primitives.WorkflowParser; +using RuntimeWorkflowYamlStructureException = Aevatar.Workflow.Core.Primitives.WorkflowYamlStructureException; using RuntimeWorkflowValidator = Aevatar.Workflow.Core.Validation.WorkflowValidator; namespace Aevatar.Studio.Application.Studio.Services; @@ -41,15 +42,19 @@ public ParseYamlResponse ParseYaml(ParseYamlRequest request) return new ParseYamlResponse(null, null, parse.Findings); } + var runtimeFindings = ValidateRuntimeYaml( + request.Yaml, + request.AvailableStepTypes); var findings = MergeFindings( parse.Findings, ValidateDocument( parse.Document, request.AvailableWorkflowNames, request.AvailableStepTypes), - ValidateRuntimeYaml( - request.Yaml, - request.AvailableStepTypes)); + runtimeFindings); + + if (runtimeFindings.Any(static finding => finding.Code is "yaml_structure" or "unknown_field")) + return new ParseYamlResponse(null, null, findings); return new ParseYamlResponse( parse.Document, @@ -167,6 +172,13 @@ private IReadOnlyList ValidateRuntimeYaml( code: "runtime_validation")) .ToList(); } + catch (RuntimeWorkflowYamlStructureException ex) + { + return + [ + ValidationFinding.Error(ex.Path, ex.Message, code: ex.Code), + ]; + } catch (Exception ex) { return diff --git a/src/Aevatar.Studio.Infrastructure/Serialization/YamlWorkflowDocumentService.cs b/src/Aevatar.Studio.Infrastructure/Serialization/YamlWorkflowDocumentService.cs index 888c5bd9d3..69afa1a5cd 100644 --- a/src/Aevatar.Studio.Infrastructure/Serialization/YamlWorkflowDocumentService.cs +++ b/src/Aevatar.Studio.Infrastructure/Serialization/YamlWorkflowDocumentService.cs @@ -6,8 +6,10 @@ using YamlDotNet.Serialization; using YamlDotNet.Serialization.NamingConventions; using YamlDotNet.RepresentationModel; +using WorkflowParser = Aevatar.Workflow.Core.Primitives.WorkflowParser; using WorkflowYamlResourceGuard = Aevatar.Workflow.Core.Primitives.WorkflowYamlResourceGuard; using WorkflowYamlResourceLimitException = Aevatar.Workflow.Core.Primitives.WorkflowYamlResourceLimitException; +using WorkflowYamlStructureException = Aevatar.Workflow.Core.Primitives.WorkflowYamlStructureException; namespace Aevatar.Studio.Infrastructure.Serialization; @@ -68,6 +70,7 @@ public WorkflowParseResult Parse(string yaml) stream = new YamlStream(); using var reader = new StringReader(yaml); stream.Load(reader); + WorkflowParser.ValidateStructure(stream); } catch (WorkflowYamlResourceLimitException exception) { @@ -81,14 +84,14 @@ public WorkflowParseResult Parse(string yaml) null, [ValidationFinding.Error("/", $"YAML syntax error: {exception.Message}", code: "yaml_syntax")]); } - - if (stream.Documents.Count == 0 || stream.Documents[0].RootNode is not YamlMappingNode root) + catch (WorkflowYamlStructureException exception) { return new WorkflowParseResult( null, - [ValidationFinding.Error("/", "Workflow YAML must contain a root mapping.", code: "yaml_root_mapping")]); + [ValidationFinding.Error(exception.Path, exception.Message, code: exception.Code)]); } + var root = (YamlMappingNode)stream.Documents[0].RootNode; ReportUnknownKeys(root, _profile.AllowedRootFields, string.Empty, findings); var document = new WorkflowDocument diff --git a/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.Presentation.cs b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.Presentation.cs new file mode 100644 index 0000000000..f34f755f01 --- /dev/null +++ b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.Presentation.cs @@ -0,0 +1,158 @@ +using System.Text.Json; +using Aevatar.Foundation.Abstractions.Interactions; +using Google.Protobuf.Reflection; +using YamlDotNet.RepresentationModel; + +namespace Aevatar.Workflow.Core.Primitives; + +public sealed partial class WorkflowParser +{ + private static bool ValidateSpecialField(Type contract, string key, YamlNode node, string path) + { + if (contract != typeof(RawRole) && contract != typeof(RawStep) && contract != typeof(RawStepPresentation)) + return false; + + return ValidatePresentationField(key, node, path, inlinePresentation: contract == typeof(RawStepPresentation)); + } + + private static void ValidateReservedParameters(YamlMappingNode parameters, string path) + { + var fields = new HashSet(StringComparer.Ordinal); + foreach (var entry in parameters.Children) + { + var key = ReadKey(entry.Key, path); + var fieldPath = AppendPath(path, key); + if (ValidatePresentationField(key, entry.Value, fieldPath)) + { + var canonical = key switch + { + "allowedTools" => "allowed_tools", + "toolSets" => "tool_sets", + "interactionSpec" => "interaction_spec", + "interactionTemplateSpec" => "interaction_template_spec", + _ => key, + }; + if (!fields.Add(canonical)) + throw StructureError(fieldPath, $"Duplicate parameter aliases for '{canonical}'."); + } + } + } + + private static bool ValidatePresentationField(string key, YamlNode node, string path, bool inlinePresentation = false) + { + switch (key) + { + case "allowed_tools": + case "allowedTools": + case "tool_sets": + case "toolSets": + ValidateToolScope(node, path); + return true; + case "interaction_spec": + case "interactionSpec": + ValidateInteraction(node, InteractionSpec.Descriptor, path); + return true; + case "interaction_template_spec": + case "interactionTemplateSpec": + ValidateInteraction(node, InteractionTemplateSpec.Descriptor, path); + return true; + case "actions" when inlinePresentation: + case "fields" when inlinePresentation: + case "cards" when inlinePresentation: + ValidateInteractionField(node, InteractionSpec.Descriptor.FindFieldByName(key), path); + return true; + default: + return false; + } + } + + private static void ValidateToolScope(YamlNode node, string path) + { + if (IsNullNode(node)) + return; + if (node is YamlScalarNode scalar) + { + var text = scalar.Value?.Trim(); + if (text?.StartsWith("[", StringComparison.Ordinal) == true || text?.StartsWith("{", StringComparison.Ordinal) == true) + ValidateNode(DecodeJsonNode(node, path), typeof(List), path, allowNull: false); + return; + } + ValidateNode(node, typeof(List), path, allowNull: false); + } + + private static void ValidateInteraction(YamlNode node, MessageDescriptor contract, string path) + { + if (IsNullNode(node)) + return; + node = DecodeJsonNode(node, path); + if (node is not YamlMappingNode mapping) + throw StructureError(path, "Expected an interaction mapping or a JSON object string."); + + var fields = new HashSet(); + foreach (var entry in mapping.Children) + { + var key = ReadKey(entry.Key, path); + var field = contract.Fields.InDeclarationOrder().FirstOrDefault(candidate => + KeyCandidates(candidate.Name).Contains(key, StringComparer.OrdinalIgnoreCase) || + (candidate.Name == "disabled" && KeyCandidates("is_disabled").Contains(key, StringComparer.OrdinalIgnoreCase))); + var fieldPath = AppendPath(path, key); + if (field is null) + throw new WorkflowYamlStructureException(fieldPath, $"Unknown interaction field '{key}'.", code: "unknown_field"); + if (!fields.Add(field.FieldNumber)) + throw StructureError(fieldPath, $"Duplicate interaction aliases for '{field.Name}'."); + ValidateInteractionField(entry.Value, field, fieldPath); + } + } + + private static void ValidateInteractionField(YamlNode node, FieldDescriptor field, string path) + { + if (IsNullNode(node)) + return; + if (field.IsMap) + { + node = DecodeJsonNode(node, path); + ValidateNode(node, typeof(Dictionary), path, allowNull: false); + } + else if (field.IsRepeated) + { + node = DecodeJsonNode(node, path); + if (node is not YamlSequenceNode sequence) + throw StructureError(path, "Expected a list of interaction objects."); + for (var index = 0; index < sequence.Children.Count; index++) + { + var item = sequence.Children[index]; + var itemPath = AppendPath(path, index.ToString()); + if (IsNullNode(item)) + throw StructureError(itemPath, "A null interaction list entry is not allowed."); + ValidateInteraction(item, field.MessageType, itemPath); + } + } + else + { + ValidateNode(node, typeof(string), path, allowNull: true); + if (field.FieldType == FieldType.Bool && + node is YamlScalarNode scalar && !bool.TryParse(scalar.Value, out _)) + throw StructureError(path, "Expected a boolean value (true or false)."); + } + } + + private static YamlNode DecodeJsonNode(YamlNode node, string path) + { + if (node is not YamlScalarNode scalar) + return node; + try + { + using var document = JsonDocument.Parse(scalar.Value ?? string.Empty); + var json = document.RootElement.GetRawText(); + WorkflowYamlResourceGuard.Validate(json); + var stream = new YamlStream(); + using var reader = new StringReader(json); + stream.Load(reader); + return stream.Documents[0].RootNode; + } + catch (JsonException) + { + throw StructureError(path, "Expected structured YAML or a valid JSON object/array string."); + } + } +} diff --git a/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.cs b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.cs new file mode 100644 index 0000000000..2df6d95767 --- /dev/null +++ b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.Structure.cs @@ -0,0 +1,282 @@ +using System.Reflection; +using Aevatar.Workflow.Abstractions.Workflows; +using YamlDotNet.Core; +using YamlDotNet.Core.Events; +using YamlDotNet.RepresentationModel; +using YamlDotNet.Serialization; +using YamlDotNet.Serialization.NamingConventions; + +namespace Aevatar.Workflow.Core.Primitives; + +public sealed partial class WorkflowParser +{ + private static readonly string[] BranchConditionFields = ["condition", "when", "case", "label", "if"]; + private static readonly string[] BranchTargetFields = ["next", "to", "target", "step"]; + + /// + /// Checks the original YAML nodes before defaults or normalization can discard invalid input. + /// Callers loading their own stream must apply first. + /// + public static void ValidateStructure(YamlStream stream) + { + ArgumentNullException.ThrowIfNull(stream); + if (stream.Documents.Count == 0) + throw new WorkflowYamlStructureException("/", "YAML 为空"); + if (stream.Documents.Count != 1) + throw new WorkflowYamlStructureException("/", "Workflow YAML must contain exactly one document."); + + var root = stream.Documents[0].RootNode; + if (IsNullNode(root)) + throw new WorkflowYamlStructureException("/", "YAML 为空"); + ValidateNode(root, typeof(Raw), string.Empty, allowNull: false); + } + + private static void ValidateNode(YamlNode node, Type contract, string path, bool allowNull) + { + if (IsNullNode(node)) + { + if (!allowNull) + throw StructureError(path, "A null value is not allowed here."); + return; + } + + contract = Nullable.GetUnderlyingType(contract) ?? contract; + if (contract == typeof(object)) + { + ValidateOpenValue(node, path); + return; + } + + if (node is YamlMappingNode keyedMapping) + ValidateMappingKeys(keyedMapping, path); + + if (contract == typeof(string) || contract.IsValueType) + { + if (node is not YamlScalarNode) + throw StructureError(path, "Expected a scalar value."); + return; + } + + if (contract.IsGenericType && contract.GetGenericTypeDefinition() == typeof(List<>)) + { + if (node is not YamlSequenceNode sequence) + throw StructureError(path, "Expected a list."); + var elementContract = contract.GetGenericArguments()[0]; + for (var index = 0; index < sequence.Children.Count; index++) + ValidateNode(sequence.Children[index], elementContract, AppendPath(path, index.ToString()), allowNull: false); + return; + } + + if (contract.IsGenericType && contract.GetGenericTypeDefinition() == typeof(Dictionary<,>)) + { + if (node is not YamlMappingNode dictionary) + throw StructureError(path, "Expected a mapping."); + foreach (var entry in dictionary.Children) + { + var key = ReadKey(entry.Key, path); + ValidateNode(entry.Value, contract.GetGenericArguments()[1], AppendPath(path, key), allowNull: true); + } + return; + } + + if (node is not YamlMappingNode mapping) + throw StructureError(path, "Expected a mapping."); + + var properties = contract.GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(static property => property.CanWrite && property.GetCustomAttribute() is null) + .ToDictionary(static property => + property.GetCustomAttribute()?.Alias ?? + UnderscoredNamingConvention.Instance.Apply(property.Name), StringComparer.Ordinal); + + foreach (var entry in mapping.Children) + { + var key = ReadKey(entry.Key, path); + var fieldPath = AppendPath(path, key); + if (contract == typeof(Raw) && !WorkflowYamlRootSchema.IsAcceptedRootField(key)) + { + throw new WorkflowYamlStructureException(fieldPath, + $"Unsupported workflow YAML root field '{key}'. Allowed root fields: {WorkflowYamlRootSchema.FormatAcceptedRootFields()}.", + code: "unknown_field"); + } + if (!properties.TryGetValue(key, out var property)) + throw new WorkflowYamlStructureException(fieldPath, $"Unknown field '{key}'.", code: "unknown_field"); + + if (contract == typeof(RawStep) && key == "branches") + ValidateBranches(entry.Value, fieldPath); + else if (ValidateSpecialField(contract, key, entry.Value, fieldPath)) + continue; + else + ValidateNode(entry.Value, property.PropertyType, fieldPath, allowNull: !property.PropertyType.IsValueType || Nullable.GetUnderlyingType(property.PropertyType) is not null); + + if (contract == typeof(RawStep) && key == "parameters" && entry.Value is YamlMappingNode parameters) + ValidateReservedParameters(parameters, fieldPath); + } + + if (contract == typeof(Raw)) + RequireIdentity(mapping, "name", path, "缺少 name"); + else if (contract == typeof(RawRole)) + RequireIdentity(mapping, "id", path, "role 缺 id"); + else if (contract == typeof(RawStep)) + RequireIdentity(mapping, "id", path, "step 缺 id"); + } + + private static void RequireIdentity(YamlMappingNode mapping, string key, string path, string message) + { + var node = mapping.Children.FirstOrDefault(entry => + entry.Key is YamlScalarNode scalar && scalar.Value == key).Value; + if (node is null || IsNullNode(node)) + throw StructureError(AppendPath(path, key), message); + } + + private static void ValidateOpenValue(YamlNode node, string path) + { + if (node is YamlMappingNode mapping) + { + ValidateMappingKeys(mapping, path); + foreach (var entry in mapping.Children) + { + var key = ReadKey(entry.Key, path); + ValidateOpenValue(entry.Value, AppendPath(path, key)); + } + } + else if (node is YamlSequenceNode sequence) + { + for (var index = 0; index < sequence.Children.Count; index++) + ValidateOpenValue(sequence.Children[index], AppendPath(path, index.ToString())); + } + } + + private static void ValidateBranches(YamlNode node, string path) + { + if (IsNullNode(node)) + return; + + if (node is YamlMappingNode mapping) + { + ValidateMappingKeys(mapping, path); + foreach (var entry in mapping.Children) + { + var key = ReadKey(entry.Key, path); + ValidateBranchTarget(entry.Value, AppendPath(path, key)); + } + return; + } + + if (node is not YamlSequenceNode sequence) + throw StructureError(path, "`branches` must be a mapping or a list."); + + var conditions = new HashSet(StringComparer.Ordinal); + for (var index = 0; index < sequence.Children.Count; index++) + { + var branchPath = AppendPath(path, index.ToString()); + if (sequence.Children[index] is not YamlMappingNode branch || IsNullNode(sequence.Children[index])) + throw StructureError(branchPath, "Each branch entry must be a mapping."); + + ValidateMappingKeys(branch, branchPath); + foreach (var entry in branch.Children) + { + var key = ReadKey(entry.Key, branchPath); + if (!BranchConditionFields.Contains(key, StringComparer.OrdinalIgnoreCase) && + !BranchTargetFields.Contains(key, StringComparer.OrdinalIgnoreCase)) + throw new WorkflowYamlStructureException(AppendPath(branchPath, key), $"Unknown branch field '{key}'.", code: "unknown_field"); + } + + var condition = ReadBranchAlias(branch, BranchConditionFields, branchPath, "condition"); + if (!conditions.Add(condition)) + throw StructureError(branchPath, $"Duplicate branch condition '{condition}'."); + ReadBranchAlias(branch, BranchTargetFields, branchPath, "target"); + } + } + + private static string ReadBranchAlias(YamlMappingNode mapping, string[] aliases, string path, string name) + { + string? result = null; + foreach (var entry in mapping.Children) + { + var key = ReadKey(entry.Key, path); + if (!aliases.Contains(key, StringComparer.OrdinalIgnoreCase)) + continue; + var value = ReadRequiredScalar(entry.Value, AppendPath(path, key)); + if (result is not null) + throw StructureError(path, $"A branch must declare exactly one {name} field."); + result = value; + } + return result ?? throw StructureError(path, $"A branch is missing its {name}."); + } + + private static void ValidateBranchTarget(YamlNode node, string path) + { + if (node is not YamlMappingNode mapping) + { + ReadRequiredScalar(node, path); + return; + } + ValidateMappingKeys(mapping, path); + foreach (var entry in mapping.Children) + { + var key = ReadKey(entry.Key, path); + if (!BranchTargetFields.Contains(key, StringComparer.OrdinalIgnoreCase)) + throw new WorkflowYamlStructureException(AppendPath(path, key), $"Unknown branch target field '{key}'.", code: "unknown_field"); + } + ReadBranchAlias(mapping, BranchTargetFields, path, "target"); + } + + private static string ReadRequiredScalar(YamlNode node, string path) + { + if (node is not YamlScalarNode scalar || IsNullNode(node) || string.IsNullOrWhiteSpace(scalar.Value)) + throw StructureError(path, "Expected a non-empty scalar value."); + return scalar.Value; + } + + private static void ValidateMappingKeys(YamlMappingNode mapping, string path) + { + var keys = new HashSet(StringComparer.Ordinal); + foreach (var entry in mapping.Children) + { + var key = ReadKey(entry.Key, path); + if (!keys.Add(key)) + throw StructureError(AppendPath(path, key), $"Duplicate mapping key '{key}'."); + } + } + + private static string ReadKey(YamlNode node, string path) + { + ReadRequiredScalar(node, path); + return ((YamlScalarNode)node).Value!; + } + + private static bool IsNullNode(YamlNode node) => + node is YamlScalarNode scalar && + (scalar.Tag == "tag:yaml.org,2002:null" || + ((scalar.Tag.IsEmpty || scalar.Tag.IsNonSpecific) && scalar.Style == YamlDotNet.Core.ScalarStyle.Plain && + (scalar.Value is null or "" or "~" or "null" or "Null" or "NULL"))); + + private static string AppendPath(string path, string key) => + $"{path}/{key.Replace("~", "~0", StringComparison.Ordinal).Replace("/", "~1", StringComparison.Ordinal)}"; + + private static WorkflowYamlStructureException StructureError(string path, string message) => + new(string.IsNullOrEmpty(path) ? "/" : path, message); + + // YamlDotNet's default null reader also consumes plain !!str null/~ scalars. + // Keep mapping consistent with the original AST's explicit non-null tags. + private sealed class ExplicitTagAwareNullNodeDeserializer(INodeDeserializer inner) : INodeDeserializer + { + public bool Deserialize( + IParser parser, + Type expectedType, + Func nestedObjectDeserializer, + out object? value, + ObjectDeserializer rootDeserializer) + { + if (parser.Accept(out var scalar) && + !scalar.Tag.IsEmpty && !scalar.Tag.IsNonSpecific && + scalar.Tag != "tag:yaml.org,2002:null") + { + value = null; + return false; + } + + return inner.Deserialize(parser, expectedType, nestedObjectDeserializer, out value, rootDeserializer); + } + } +} diff --git a/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.cs b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.cs index 106df12a13..16bd232096 100644 --- a/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.cs +++ b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowParser.cs @@ -4,6 +4,7 @@ // ───────────────────────────────────────────────────────────── using YamlDotNet.Serialization; +using YamlDotNet.Serialization.NodeDeserializers; using YamlDotNet.Serialization.NamingConventions; using YamlDotNet.RepresentationModel; using Aevatar.Foundation.Abstractions.Interactions; @@ -19,10 +20,14 @@ namespace Aevatar.Workflow.Core.Primitives; /// /// YAML 工作流解析器。使用 snake_case 命名约定将 YAML 解析为强类型工作流定义。 /// -public sealed class WorkflowParser +public sealed partial class WorkflowParser { private static readonly IDeserializer D = new DeserializerBuilder() - .WithNamingConvention(UnderscoredNamingConvention.Instance).Build(); + .WithNamingConvention(UnderscoredNamingConvention.Instance) + .WithNodeDeserializer( + inner => new ExplicitTagAwareNullNodeDeserializer(inner), + registration => registration.InsteadOf()) + .Build(); private static readonly (string Key, Func Getter)[] RootParameterMappings = [ ("workers", static step => step.Workers), @@ -87,7 +92,10 @@ private static readonly (string Key, Func Getter)[] RootParame public WorkflowDefinition Parse(string yaml) { WorkflowYamlResourceGuard.Validate(yaml); - ValidateRootSchema(yaml); + var stream = new YamlStream(); + using (var reader = new StringReader(yaml)) + stream.Load(reader); + ValidateStructure(stream); var raw = D.Deserialize(yaml) ?? throw new InvalidOperationException("YAML 为空"); return new WorkflowDefinition { @@ -104,29 +112,6 @@ public WorkflowDefinition Parse(string yaml) }; } - private static void ValidateRootSchema(string yaml) - { - YamlStream stream = new(); - using var reader = new StringReader(yaml); - stream.Load(reader); - - if (stream.Documents.Count == 0) - return; - - if (stream.Documents[0].RootNode is not YamlMappingNode root) - return; - - foreach (var key in root.Children.Keys.OfType()) - { - var rootField = key.Value; - if (WorkflowYamlRootSchema.IsAcceptedRootField(rootField)) - continue; - - throw new InvalidOperationException( - $"Unsupported workflow YAML root field '{rootField}'. Allowed root fields: {WorkflowYamlRootSchema.FormatAcceptedRootFields()}."); - } - } - private static RoleDefinition MapRole(RawRole role) { // Refactor (iter30/cluster-030-workflow-step-raw-actor-lifecycle): diff --git a/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlStructureException.cs b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlStructureException.cs new file mode 100644 index 0000000000..2309614a30 --- /dev/null +++ b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlStructureException.cs @@ -0,0 +1,18 @@ +namespace Aevatar.Workflow.Core.Primitives; + +/// +/// A workflow YAML shape error found before mapping or normalization. +/// +public sealed class WorkflowYamlStructureException : InvalidOperationException +{ + public WorkflowYamlStructureException(string path, string message, string code = "yaml_structure") + : base($"{message} Path: {path}.") + { + Path = path; + Code = code; + } + + public string Path { get; } + + public string Code { get; } +} diff --git a/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlValidatorImpl.cs b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlValidatorImpl.cs index 2b93d1d79a..4d150f26b5 100644 --- a/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlValidatorImpl.cs +++ b/src/workflow/Aevatar.Workflow.Core/Primitives/WorkflowYamlValidatorImpl.cs @@ -23,6 +23,11 @@ public WorkflowYamlValidationResult Validate(string yaml) Description: def.Description, Diagnostics: []); } + catch (WorkflowYamlStructureException ex) + { + return new(false, null, null, 0, 0, null, + [new("error", ex.Message, Field: ex.Path)]); + } catch (Exception ex) { return new(false, null, null, 0, 0, null, diff --git a/test/Aevatar.Studio.Tests/EditorControllerSerializationTests.cs b/test/Aevatar.Studio.Tests/EditorControllerSerializationTests.cs index 898af0563b..16fb2b27ef 100644 --- a/test/Aevatar.Studio.Tests/EditorControllerSerializationTests.cs +++ b/test/Aevatar.Studio.Tests/EditorControllerSerializationTests.cs @@ -422,7 +422,43 @@ public async Task ParseYaml_ShouldRejectNonMappingCapabilitySelector(string sele var body = await response.Content.ReadAsStringAsync(); response.StatusCode.Should().Be(HttpStatusCode.OK, body); body.Should().Contain($"\"path\":\"/steps/0/capability/{selector}\""); - body.Should().Contain("\"code\":\"invalid_field\""); + body.Should().Contain("\"code\":\"yaml_structure\""); + } + + [Theory] + [InlineData("true: finish\nfalse: {}", "/steps/0/branches/false")] + [InlineData("- condition: true\n next: finish\n- condition: false", "/steps/0/branches/1")] + public async Task ParseYaml_WhenOnlyOneBranchIsMalformed_ShouldRejectEntireDocument( + string branches, + string expectedPath) + { + using var host = await StartHostAsync(); + var client = host.GetTestClient(); + var indentedBranches = string.Join('\n', branches.Split('\n').Select(line => $" {line}")); + + using var response = await client.PostAsJsonAsync("/api/editor/parse-yaml", new + { + yaml = $$""" + name: mixed_branches + steps: + - id: choose + type: conditional + branches: + {{indentedBranches}} + - id: finish + type: assign + """, + availableStepTypes = new[] { "conditional", "assign" }, + }); + + var body = await response.Content.ReadAsStringAsync(); + response.StatusCode.Should().Be(HttpStatusCode.OK, body); + using var json = JsonDocument.Parse(body); + json.RootElement.TryGetProperty("document", out _).Should().BeFalse(); + json.RootElement.TryGetProperty("graph", out _).Should().BeFalse(); + json.RootElement.GetProperty("findings").EnumerateArray().Should().ContainSingle(finding => + finding.GetProperty("path").GetString() == expectedPath && + finding.GetProperty("code").GetString() == "yaml_structure"); } private static object BuildPlainParameterRequest() => new diff --git a/test/Aevatar.Studio.Tests/WorkflowEditorServiceStructureTests.cs b/test/Aevatar.Studio.Tests/WorkflowEditorServiceStructureTests.cs new file mode 100644 index 0000000000..8cf3189ecc --- /dev/null +++ b/test/Aevatar.Studio.Tests/WorkflowEditorServiceStructureTests.cs @@ -0,0 +1,170 @@ +using Aevatar.Studio.Application.Studio.Abstractions; +using Aevatar.Studio.Application.Studio.Contracts; +using Aevatar.Studio.Application.Studio.Services; +using Aevatar.Studio.Domain.Studio.Compatibility; +using Aevatar.Studio.Domain.Studio.Models; +using Aevatar.Studio.Domain.Studio.Services; +using Aevatar.Studio.Infrastructure.Serialization; +using FluentAssertions; + +namespace Aevatar.Studio.Tests; + +public sealed class WorkflowEditorServiceStructureTests +{ + [Theory] + [InlineData("true: finish\nfalse: {}", "/steps/0/branches/false")] + [InlineData("- condition: true\n next: finish\n- condition: false", "/steps/0/branches/1")] + public void ParseYaml_WhenBranchTargetIsMissing_ShouldRejectBeforeCreatingDocument( + string branches, + string expectedPath) + { + var service = CreateEditor(); + + var result = service.ParseYaml(new ParseYamlRequest(BuildWorkflowWithBranches(branches))); + + result.Document.Should().BeNull(); + result.Graph.Should().BeNull(); + result.Findings.Should().ContainSingle(finding => + finding.Level == ValidationLevel.Error && + finding.Code == "yaml_structure" && + finding.Path == expectedPath); + } + + [Fact] + public void ParseYaml_WhenChildIsNull_ShouldReturnStructureFindingInsteadOfPartialDocument() + { + var service = CreateEditor(); + + var result = service.ParseYaml(new ParseYamlRequest(""" + name: null_child + steps: + - id: parent + type: assign + children: + - null + """)); + + result.Document.Should().BeNull(); + result.Graph.Should().BeNull(); + result.Findings.Should().ContainSingle(finding => + finding.Level == ValidationLevel.Error && + finding.Code == "yaml_structure" && + finding.Path == "/steps/0/children/0"); + result.Findings.Should().NotContain(finding => finding.Code == "runtime_validation"); + } + + [Fact] + public void ParseYaml_WhenYamlSyntaxIsInvalid_ShouldKeepSyntaxDiagnosticDistinct() + { + var service = CreateEditor(); + + var result = service.ParseYaml(new ParseYamlRequest("name: invalid\nsteps: [")); + + result.Document.Should().BeNull(); + result.Graph.Should().BeNull(); + result.Findings.Should().ContainSingle(finding => finding.Code == "yaml_syntax"); + } + + [Theory] + [InlineData("true: { next: finish }\nfalse: finish")] + [InlineData("- condition: true\n next: finish\n- condition: false\n next: finish")] + public void ParseAndSerializeYaml_WhenBranchesAndComplexParametersAreValid_ShouldPreserveThem(string branches) + { + var service = CreateEditor(); + + var parsed = service.ParseYaml(new ParseYamlRequest(BuildWorkflowWithBranches(branches))); + + parsed.Document.Should().NotBeNull(); + parsed.Findings.Should().NotContain(finding => finding.Level == ValidationLevel.Error); + parsed.Document!.Steps[0].Branches.Should().BeEquivalentTo(new Dictionary + { + ["true"] = "finish", + ["false"] = "finish", + }); + var originalValue = parsed.Document.Steps[1].Parameters["value"].Should() + .BeOfType().Which; + + var serialized = service.SerializeYaml(new SerializeYamlRequest(parsed.Document)); + var roundTrip = service.ParseYaml(new ParseYamlRequest(serialized.Yaml)); + + serialized.Findings.Should().NotContain(finding => finding.Level == ValidationLevel.Error); + roundTrip.Findings.Should().NotContain(finding => finding.Level == ValidationLevel.Error); + roundTrip.Document!.Steps[0].Branches.Should().BeEquivalentTo(parsed.Document.Steps[0].Branches); + var roundTripValue = roundTrip.Document.Steps[1].Parameters["value"].Should() + .BeOfType().Which; + roundTripValue.Properties.Keys.Should().BeEquivalentTo("items", "options"); + var items = roundTripValue.Properties["items"].Should().BeOfType().Which; + items.Items.Should().HaveCount(3); + items.Items[0].Should().BeOfType().Which.Scalar.Should().Be("one"); + var nested = items.Items[2].Should().BeOfType().Which; + nested.Properties.Should().ContainSingle(); + nested.Properties["nested"].Should().BeOfType().Which.Scalar.Should().Be("two"); + var options = roundTripValue.Properties["options"].Should().BeOfType().Which; + options.Properties.Should().ContainSingle(); + options.Properties["enabled"].Should().BeOfType().Which.Scalar.Should().Be("true"); + roundTripValue.ToWorkflowScalarString().Should().Be(originalValue.ToWorkflowScalarString()); + } + + [Fact] + public void ParseYaml_WhenDocumentAdapterMissesInvalidStructure_ShouldKeepRuntimeStructurePathAndCode() + { + var service = CreateEditor(new PermissiveDocumentService()); + + var result = service.ParseYaml(new ParseYamlRequest( + BuildWorkflowWithBranches("true: finish\nfalse: {}"))); + + result.Document.Should().BeNull(); + result.Graph.Should().BeNull(); + result.Findings.Should().ContainSingle(finding => + finding.Path == "/steps/0/branches/false" && + finding.Code == "yaml_structure" && + finding.Level == ValidationLevel.Error); + result.Findings.Should().NotContain(finding => finding.Code == "runtime_validation"); + } + + private static WorkflowEditorService CreateEditor(IWorkflowYamlDocumentService? documentService = null) + { + var profile = WorkflowCompatibilityProfile.AevatarV1; + return new WorkflowEditorService( + documentService ?? new YamlWorkflowDocumentService(profile), + new WorkflowDocumentNormalizer(profile), + new WorkflowValidator(profile), + new WorkflowGraphMapper(profile), + new TextDiffService()); + } + + private static string BuildWorkflowWithBranches(string branches) + { + var indentedBranches = string.Join('\n', branches.Split('\n').Select(line => $" {line}")); + return $$""" + name: branch_structure + roles: [] + steps: + - id: choose + type: conditional + branches: + {{indentedBranches}} + - id: finish + type: assign + parameters: + target: result + value: + items: [one, null, { nested: two }] + options: + enabled: true + """; + } + + private sealed class PermissiveDocumentService : IWorkflowYamlDocumentService + { + public WorkflowParseResult Parse(string yaml) => new( + new WorkflowDocument + { + Name = "adapter_document", + Steps = [new StepModel { Id = "finish", Type = "assign" }], + }, + []); + + public string Serialize(WorkflowDocument document) => throw new NotSupportedException(); + } +} diff --git a/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserCoverageTests.cs b/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserCoverageTests.cs index 66ec88e6d0..c5bd19d54c 100644 --- a/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserCoverageTests.cs +++ b/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserCoverageTests.cs @@ -73,7 +73,6 @@ public void Parse_WhenBranchesUseDictionaryAndListForms_ShouldNormalizeTargets() next: done - when: retry to: fallback - - if: ignored - id: fallback type: assign parameters: diff --git a/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserPresentationStructureTests.cs b/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserPresentationStructureTests.cs new file mode 100644 index 0000000000..7686c2db73 --- /dev/null +++ b/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserPresentationStructureTests.cs @@ -0,0 +1,227 @@ +using Aevatar.Workflow.Core.Primitives; +using FluentAssertions; + +namespace Aevatar.Workflow.Core.Tests.Primitives; + +public sealed class WorkflowParserPresentationStructureTests +{ + [Theory] + [MemberData(nameof(InvalidToolScopes))] + public void Parse_WhenToolScopeContainsObjectsOrNullEntries_ShouldRejectAtTheSourcePath( + string yaml, + string expectedPath) + { + AssertStructureFailure(yaml, expectedPath); + } + + [Theory] + [MemberData(nameof(ValidToolScopes))] + public void Parse_WhenToolScopeUsesSupportedScalarOrListForms_ShouldPreserveTheNames( + string owner, + string field, + string value, + string[] expectedNames) + { + var workflow = new WorkflowParser().Parse(BuildToolScopeYaml(owner, field, value)); + var scope = owner == "role" ? workflow.Roles[0].AgentToolScope : workflow.Steps[0].AgentToolScope; + var isAllowedTools = field is "allowed_tools" or "allowedTools"; + + scope.Should().NotBeNull(); + scope!.RestrictAllowedToolNames.Should().Be(isAllowedTools); + scope.RestrictToolSets.Should().Be(!isAllowedTools); + if (isAllowedTools) + scope.AllowedToolNames.Should().Equal(expectedNames); + else + scope.ToolSetRefs.Should().Equal(expectedNames); + + if (owner == "parameters") + workflow.Steps[0].Parameters.Should().NotContainKey(field); + } + + [Theory] + [InlineData("interaction_spec: []", "/steps/0/interaction_spec")] + [InlineData("interaction_spec: 42", "/steps/0/interaction_spec")] + [InlineData("interaction_template_spec: []", "/steps/0/interaction_template_spec")] + [InlineData("presentation: { interaction_spec: [] }", "/steps/0/presentation/interaction_spec")] + [InlineData("parameters: { interaction_spec: [] }", "/steps/0/parameters/interaction_spec")] + [InlineData("parameters: { interactionSpec: [] }", "/steps/0/parameters/interactionSpec")] + [InlineData("interaction_spec: { title: Review, actions: [null] }", "/steps/0/interaction_spec/actions/0")] + [InlineData("interaction_spec: { title: Review, fields: [invalid] }", "/steps/0/interaction_spec/fields/0")] + [InlineData("interaction_spec: { title: Review, cards: [null] }", "/steps/0/interaction_spec/cards/0")] + [InlineData("presentation: { title: Review, actions: [invalid] }", "/steps/0/presentation/actions/0")] + public void Parse_WhenInteractionPresentationHasTheWrongShape_ShouldRejectBeforeDiscardingIt( + string fields, + string expectedPath) + { + AssertStructureFailure($"name: bad_presentation\nsteps:\n - id: notify\n type: notify\n {fields}\n", expectedPath); + } + + [Theory] + [InlineData("{ title: Review, actions: [{ action_id: approve, label: Approve }] }")] + [InlineData("'{\"title\":\"Review\",\"actions\":[{\"action_id\":\"approve\",\"label\":\"Approve\"}]}'")] + public void Parse_WhenInteractionSpecUsesAYamlMappingOrJsonObjectString_ShouldPreserveItsContent(string spec) + { + var yaml = $"name: valid_presentation\nsteps:\n - id: notify\n type: notify\n interaction_spec: {spec}\n"; + + var workflow = new WorkflowParser().Parse(yaml); + + var interaction = workflow.Steps[0].Presentation?.InteractionSpec; + interaction.Should().NotBeNull(); + interaction!.Title.Should().Be("Review"); + interaction.Actions.Should().ContainSingle().Which.ActionId.Should().Be("approve"); + } + + [Theory] + [InlineData("{ actions: [{ action_id: approve, id: reject }] }")] + [InlineData("'{\"actions\":[{\"action_id\":\"approve\",\"id\":\"reject\"}]}'")] + public void Parse_WhenInteractionAliasesSpecifyTheSameFieldTwice_ShouldRejectTheSecondAlias(string spec) + { + var yaml = $"name: duplicate_action_alias\nsteps:\n - id: notify\n type: notify\n interaction_spec: {spec}\n"; + + AssertStructureFailure(yaml, "/steps/0/interaction_spec/actions/0/id"); + } + + [Theory] + [InlineData("allowed_tools: [search], allowedTools: [calendar]", "allowedTools")] + [InlineData("tool_sets: [search], toolSets: [calendar]", "toolSets")] + [InlineData("interaction_spec: { title: Review }, interactionSpec: { title: Other }", "interactionSpec")] + [InlineData("interaction_template_spec: { template_id: first }, interactionTemplateSpec: { template_id: second }", "interactionTemplateSpec")] + public void Parse_WhenParameterAliasesSpecifyTheSameReservedFieldTwice_ShouldRejectTheSecondAlias( + string parameters, + string secondAlias) + { + var yaml = $"name: duplicate_parameter_alias\nsteps:\n - id: notify\n type: notify\n parameters: {{ {parameters} }}\n"; + + AssertStructureFailure(yaml, $"/steps/0/parameters/{secondAlias}"); + } + + [Fact] + public void Parse_WhenJsonEncodedInteractionExceedsTheNodeBudget_ShouldRejectTheDecodedTree() + { + var cards = string.Join(',', Enumerable.Repeat("{\"title\":\"card\"}", WorkflowYamlResourceGuard.MaxNodes / 2)); + var yaml = $"name: oversized_interaction\nsteps:\n - id: notify\n type: notify\n interaction_spec: '{{\"cards\":[{cards}]}}'\n"; + var validateOuterYaml = () => WorkflowYamlResourceGuard.Validate(yaml); + var parse = () => new WorkflowParser().Parse(yaml); + + validateOuterYaml.Should().NotThrow("the encoded interaction is a single YAML scalar below the byte limit"); + var exception = parse.Should().Throw().Which; + exception.LimitKind.Should().Be(WorkflowYamlResourceLimitKind.Nodes); + exception.Maximum.Should().Be(WorkflowYamlResourceGuard.MaxNodes); + } + + [Theory] + [InlineData("actions: [{ action_id: approve, disabled: definitely }]", "/steps/0/interaction_spec/actions/0/disabled")] + [InlineData("actions: [{ action_id: approve, is_disabled: definitely }]", "/steps/0/interaction_spec/actions/0/is_disabled")] + [InlineData("fields: [{ title: Environment, is_short: definitely }]", "/steps/0/interaction_spec/fields/0/is_short")] + public void Parse_WhenInteractionBooleanUsesAnInvalidScalar_ShouldRejectInsteadOfDefaultingToFalse( + string fields, + string expectedPath) + { + var yaml = $"name: invalid_interaction_boolean\nsteps:\n - id: notify\n type: notify\n interaction_spec: {{ {fields} }}\n"; + + AssertStructureFailure(yaml, expectedPath); + } + + [Theory] + [InlineData("true", true)] + [InlineData("false", false)] + [InlineData("null", false)] + public void Parse_WhenInteractionBooleansAreValidOrNull_ShouldPreserveTheValueOrDefault( + string value, + bool expectedValue) + { + var yaml = $$""" + name: valid_interaction_boolean + steps: + - id: notify + type: notify + interaction_spec: + actions: + - action_id: approve + disabled: {{value}} + fields: + - title: Environment + is_short: {{value}} + """; + + var workflow = new WorkflowParser().Parse(yaml); + + var interaction = workflow.Steps[0].Presentation?.InteractionSpec; + interaction.Should().NotBeNull(); + interaction!.Actions.Should().ContainSingle().Which.Disabled.Should().Be(expectedValue); + interaction.Fields.Should().ContainSingle().Which.IsShort.Should().Be(expectedValue); + } + + [Theory] + [InlineData("disabled")] + [InlineData("is_disabled")] + [InlineData("isDisabled")] + [InlineData("IS_DISABLED")] + public void Parse_WhenDisabledUsesASupportedCaseInsensitiveAlias_ShouldPreserveItsValue(string field) + { + var yaml = $"name: disabled_alias\nsteps:\n - id: notify\n type: notify\n interaction_spec: {{ actions: [{{ id: approve, {field}: true }}] }}\n"; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Steps[0].Presentation!.InteractionSpec.Actions.Should().ContainSingle().Which.Disabled.Should().BeTrue(); + } + + public static IEnumerable InvalidToolScopes() + { + foreach (var (owner, field) in ToolScopeLocations()) + { + var path = ToolScopePath(owner, field); + yield return [BuildToolScopeYaml(owner, field, "{ name: search }"), path]; + yield return [BuildToolScopeYaml(owner, field, "[{ name: search }]"), $"{path}/0"]; + yield return [BuildToolScopeYaml(owner, field, "[null]"), $"{path}/0"]; + } + } + + public static IEnumerable ValidToolScopes() + { + foreach (var (owner, field) in ToolScopeLocations()) + { + yield return [owner, field, "[]", Array.Empty()]; + yield return [owner, field, "search", new[] { "search" }]; + yield return [owner, field, "[search, calendar]", new[] { "search", "calendar" }]; + } + } + + private static IEnumerable<(string Owner, string Field)> ToolScopeLocations() + { + foreach (var owner in new[] { "role", "step", "parameters" }) + { + yield return (owner, "allowed_tools"); + yield return (owner, "tool_sets"); + } + yield return ("parameters", "allowedTools"); + yield return ("parameters", "toolSets"); + } + + private static string ToolScopePath(string owner, string field) => owner switch + { + "role" => $"/roles/0/{field}", + "step" => $"/steps/0/{field}", + _ => $"/steps/0/parameters/{field}", + }; + + private static string BuildToolScopeYaml(string owner, string field, string value) + { + var body = owner switch + { + "role" => $"roles: [{{ id: analyst, {field}: {value} }}]\nsteps: []", + "step" => $"steps: [{{ id: first, type: assign, {field}: {value} }}]", + _ => $"steps: [{{ id: first, type: assign, parameters: {{ {field}: {value} }} }}]", + }; + return $"name: tool_scope\n{body}\n"; + } + + private static void AssertStructureFailure(string yaml, string expectedPath) + { + var parse = () => new WorkflowParser().Parse(yaml); + + var exception = parse.Should().Throw().Which; + exception.Code.Should().Be("yaml_structure"); + exception.Path.Should().Be(expectedPath); + } +} diff --git a/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserStructureTests.cs b/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserStructureTests.cs new file mode 100644 index 0000000000..9b148291fe --- /dev/null +++ b/test/Aevatar.Workflow.Core.Tests/Primitives/WorkflowParserStructureTests.cs @@ -0,0 +1,368 @@ +using Aevatar.Workflow.Application.Abstractions.Workflows; +using Aevatar.Workflow.Core.Primitives; +using Aevatar.Workflow.Core.Validation; +using FluentAssertions; + +namespace Aevatar.Workflow.Core.Tests.Primitives; + +public sealed class WorkflowParserStructureTests +{ + [Theory] + [InlineData("oops", "/steps/0/branches")] + [InlineData("[oops]", "/steps/0/branches/0")] + [InlineData("[null]", "/steps/0/branches/0")] + [InlineData("[{ next: done }]", "/steps/0/branches/0")] + [InlineData("[{ condition: ready }]", "/steps/0/branches/0")] + [InlineData("[{ condition: '', next: done }]", "/steps/0/branches/0/condition")] + [InlineData("[{ condition: ready, next: '' }]", "/steps/0/branches/0/next")] + [InlineData("{ ready: '' }", "/steps/0/branches/ready")] + [InlineData("{ ready: null }", "/steps/0/branches/ready")] + [InlineData("{ 'a/b~c': '' }", "/steps/0/branches/a~1b~0c")] + public void Parse_WhenBranchesCannotDescribeARoute_ShouldRejectBeforeNormalization( + string branches, + string expectedPath) + { + var yaml = $$""" + name: invalid_branches + steps: + - id: first + type: assign + parameters: { target: result, value: ok } + branches: {{branches}} + - id: done + type: assign + """; + + AssertStructureFailure(yaml, expectedPath); + } + + [Fact] + public void Parse_WhenACompleteBranchListAlsoContainsAnIncompleteEntry_ShouldRejectTheBadEntry() + { + const string yaml = """ + name: mixed_branches + steps: + - id: gate + type: conditional + parameters: { condition: ready } + branches: + - condition: "true" + next: done + - condition: "false" + next: done + - if: ignored + - id: done + type: assign + """; + + AssertStructureFailure(yaml, "/steps/0/branches/2"); + } + + [Fact] + public void Parse_WhenBranchAliasesRepeatTheSameCondition_ShouldRejectInsteadOfOverwritingTheRoute() + { + const string yaml = """ + name: duplicate_branch + steps: + - id: gate + type: assign + branches: + - condition: ready + next: first + - when: ready + to: second + - id: first + type: assign + - id: second + type: assign + """; + + AssertStructureFailure(yaml, "/steps/0/branches/1"); + } + + [Theory] + [InlineData("steps: [{ id: first, branches: { [ready, retry]: done } }]", "/steps/0/branches")] + [InlineData("steps: [{ id: first, parameters: { [key, part]: value } }]", "/steps/0/parameters")] + public void Parse_WhenAMappingUsesACollectionAsItsKey_ShouldRejectAtTheMappingPath( + string body, + string expectedPath) + { + AssertStructureFailure($"name: collection_key\n{body}\n", expectedPath); + } + + [Theory] + [InlineData("roles: {}\nsteps: []", "/roles")] + [InlineData("roles: [null]\nsteps: []", "/roles/0")] + [InlineData("roles: [analyst]\nsteps: []", "/roles/0")] + [InlineData("steps: {}", "/steps")] + [InlineData("steps: [null]", "/steps/0")] + [InlineData("steps: [assign]", "/steps/0")] + [InlineData("steps: [{ id: first, children: {} }]", "/steps/0/children")] + [InlineData("steps: [{ id: first, children: [null] }]", "/steps/0/children/0")] + [InlineData("steps: [{ id: first, children: [assign] }]", "/steps/0/children/0")] + [InlineData("steps: [{ id: first, parameters: [] }]", "/steps/0/parameters")] + [InlineData("steps: [{ id: first, retry: [] }]", "/steps/0/retry")] + [InlineData("configuration: []\nsteps: []", "/configuration")] + public void Parse_WhenAContainerHasTheWrongShape_ShouldReportItsSourcePath( + string body, + string expectedPath) + { + AssertStructureFailure($"name: invalid_container\n{body}\n", expectedPath); + } + + [Theory] + [InlineData("description: {}\nsteps: []", "/description")] + [InlineData("roles: [{ id: analyst, model: [] }]\nsteps: []", "/roles/0/model")] + [InlineData("roles: [{ id: analyst, max_tokens: {} }]\nsteps: []", "/roles/0/max_tokens")] + [InlineData("steps: [{ id: {}, type: assign }]", "/steps/0/id")] + [InlineData("steps: [{ id: first, type: assign, next: {} }]", "/steps/0/next")] + [InlineData("steps: [{ id: first, type: assign, timeout_ms: {} }]", "/steps/0/timeout_ms")] + [InlineData("steps: [{ id: first, type: assign, retry: { max_attempts: [] } }]", "/steps/0/retry/max_attempts")] + public void Parse_WhenAScalarFieldContainsACollection_ShouldReportItsSourcePath( + string body, + string expectedPath) + { + AssertStructureFailure($"name: invalid_scalar\n{body}\n", expectedPath); + } + + [Theory] + [InlineData("roles: [{ id: analyst, typo: value }]\nsteps: []", "/roles/0/typo")] + [InlineData("steps: [{ id: first, typo: value }]", "/steps/0/typo")] + [InlineData("steps: [{ id: first, 'next ': done }]", "/steps/0/next ")] + [InlineData("steps: [{ id: first, 'typo/key~value': value }]", "/steps/0/typo~1key~0value")] + [InlineData("steps: [{ id: first, children: [{ id: child, typo: value }] }]", "/steps/0/children/0/typo")] + [InlineData("steps: [{ id: first, retry: { typo: value } }]", "/steps/0/retry/typo")] + public void Parse_WhenANestedFieldIsUnknown_ShouldIdentifyTheExactField( + string body, + string expectedPath) + { + AssertStructureFailure($"name: unknown_field\n{body}\n", expectedPath, "unknown_field"); + } + + [Theory] + [InlineData("!!str null", "null")] + [InlineData("!!str ~", "~")] + [InlineData("'null'", "null")] + [InlineData("'~'", "~")] + [InlineData("\"null\"", "null")] + [InlineData("\"~\"", "~")] + public void Parse_WhenNullLikeTextIsExplicitlyAString_ShouldPreserveIdentitiesAndBranchTargets( + string scalar, + string expectedValue) + { + var yaml = $$""" + name: {{scalar}} + roles: + - id: {{scalar}} + steps: + - id: gate + type: assign + branches: + ready: {{scalar}} + - id: {{scalar}} + type: assign + """; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Name.Should().Be(expectedValue); + workflow.Roles[0].Id.Should().Be(expectedValue); + workflow.Steps[1].Id.Should().Be(expectedValue); + workflow.Steps[0].Branches.Should().ContainKey("ready").WhoseValue.Should().Be(expectedValue); + WorkflowValidator.Validate(workflow).Should().BeEmpty(); + } + + [Theory] + [InlineData("steps: [{ id: first, children: [null] }]", "/steps/0/children/0")] + [InlineData("steps: [{ id: first, branches: [{ condition: ready }] }]", "/steps/0/branches/0")] + [InlineData("steps: [{ id: first, 'next ': done }]", "/steps/0/next ")] + public void Validate_WhenYamlHasAStructureError_ShouldReturnTheSourcePathInItsDiagnostic( + string body, + string expectedPath) + { + IWorkflowYamlValidator validator = new WorkflowYamlValidatorImpl(); + + var result = validator.Validate($"name: invalid_structure\n{body}\n"); + + result.Success.Should().BeFalse(); + var diagnostic = result.Diagnostics.Should().ContainSingle().Which; + diagnostic.Severity.Should().Be("error"); + diagnostic.Field.Should().Be(expectedPath); + diagnostic.Message.Should().Contain(expectedPath); + } + + [Fact] + public void Parse_WhenRequiredFieldKeysHaveExplicitStringTags_ShouldRecognizeTheirNames() + { + const string yaml = """ + !!str name: tagged_keys + roles: + - !!str id: analyst + steps: + - !!str id: first + type: assign + """; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Name.Should().Be("tagged_keys"); + workflow.Roles.Should().ContainSingle().Which.Id.Should().Be("analyst"); + workflow.Steps.Should().ContainSingle().Which.Id.Should().Be("first"); + } + + [Theory] + [InlineData("name: duplicate\n!!str name: overwritten\nsteps: []", "/name")] + [InlineData("name: duplicate\nsteps: [{ id: first, parameters: { key: one, !!str key: two } }]", "/steps/0/parameters/key")] + [InlineData("name: duplicate\nsteps: [{ id: first, branches: { ready: first, !!str ready: first } }]", "/steps/0/branches/ready")] + public void Parse_WhenKeysDifferOnlyByAnExplicitStringTag_ShouldRejectDuplicateFields(string yaml, string expectedPath) + { + AssertStructureFailure(yaml, expectedPath); + } + + [Fact] + public void Parse_WhenBranchConditionsHaveDifferentLiteralWhitespace_ShouldPreserveBothConditions() + { + const string yaml = """ + name: literal_conditions + steps: + - id: first + type: assign + branches: + - condition: ready + next: first + - condition: ' ready ' + next: first + """; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Steps[0].Branches.Should().BeEquivalentTo(new Dictionary + { + ["ready"] = "first", + [" ready "] = "first", + }); + } + + [Theory] + [InlineData("condition", "next")] + [InlineData("when", "to")] + [InlineData("case", "target")] + [InlineData("label", "step")] + [InlineData("if", "next")] + public void Parse_WhenBranchListUsesSupportedAliases_ShouldPreserveTheRoute( + string conditionField, + string targetField) + { + var yaml = $$""" + name: branch_aliases + steps: + - id: first + type: assign + branches: + - {{conditionField}}: ready + {{targetField}}: done + - id: done + type: assign + """; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Steps[0].Branches.Should().BeEquivalentTo(new Dictionary + { + ["ready"] = "done", + }); + WorkflowValidator.Validate(workflow).Should().BeEmpty(); + } + + [Theory] + [InlineData("next")] + [InlineData("to")] + [InlineData("target")] + [InlineData("step")] + public void Parse_WhenBranchMappingUsesSupportedTargetAliases_ShouldPreserveTheRoute(string targetField) + { + var yaml = $$""" + name: branch_mapping_aliases + steps: + - id: first + type: assign + branches: + ready: + {{targetField}}: done + fallback: done + - id: done + type: assign + """; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Steps[0].Branches.Should().BeEquivalentTo(new Dictionary + { + ["ready"] = "done", + ["fallback"] = "done", + }); + } + + [Fact] + public void Parse_WhenParametersContainNestedValuesAndYamlAliases_ShouldPreserveThem() + { + const string yaml = """ + name: structured_parameters + steps: + - id: first + type: assign + parameters: + target: result + value: &payload + enabled: true + items: [one, two] + copy: *payload + """; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Steps[0].Parameters["value"].Should().Be("""{"enabled":"true","items":["one","two"]}"""); + workflow.Steps[0].Parameters["copy"].Should().Be(workflow.Steps[0].Parameters["value"]); + } + + [Fact] + public void Parse_WhenOptionalFieldsAreExplicitlyNull_ShouldRetainTheirDefaults() + { + const string yaml = """ + name: optional_nulls + description: null + configuration: null + roles: null + on_failure: null + steps: + - id: first + type: assign + children: null + parameters: null + retry: null + on_error: null + branches: null + presentation: null + """; + + var workflow = new WorkflowParser().Parse(yaml); + + workflow.Description.Should().BeEmpty(); + workflow.Roles.Should().BeEmpty(); + workflow.Steps.Should().ContainSingle(); + workflow.Steps[0].Children.Should().BeNull(); + workflow.Steps[0].Branches.Should().BeNull(); + workflow.Steps[0].Parameters.Should().BeEmpty(); + WorkflowValidator.Validate(workflow).Should().BeEmpty(); + } + + private static void AssertStructureFailure(string yaml, string expectedPath, string expectedCode = "yaml_structure") + { + var parse = () => new WorkflowParser().Parse(yaml); + + var exception = parse.Should().Throw().Which; + exception.Code.Should().Be(expectedCode); + exception.Path.Should().Be(expectedPath); + exception.Message.Should().Contain(expectedPath); + } +}