diff --git a/docs/modules/workflow/analyzer.md b/docs/modules/workflow/analyzer.md new file mode 100644 index 000000000..60c1bfbad --- /dev/null +++ b/docs/modules/workflow/analyzer.md @@ -0,0 +1,184 @@ +# StellaOps.Workflow.Analyzer + +A Roslyn `DiagnosticAnalyzer` that rejects workflow C# code which cannot be serialized to canonical workflow JSON. Failures are surfaced as compile-time errors at the offending line — no runtime round-trip needed to discover that a workflow is non-canonical. + +## What the analyzer is for + +`IDeclarativeWorkflow` implementations build a `WorkflowSpec` from a fluent builder. At runtime, `WorkflowCanonicalDefinitionCompiler` turns that spec into a canonical JSON document that the engine can execute, persist, diff, and replay. + +For that to work, the spec must be **pure and declarative**: its shape can only depend on compile-time literals and the workflow builder surface, not on build-time state (time, environment, DB reads) or runtime C# semantics (imperative branching, exceptions, async). + +The analyzer enforces this at compile time. Consumer projects pick it up via a `` to `StellaOps.Workflow.Analyzer.csproj` (with `OutputItemType="Analyzer"`) or via an `` item pointing at the built DLL. Once referenced, every `IDeclarativeWorkflow` class in the consumer compilation is checked on every build. + +## Activation + +The analyzer activates **only when the compilation references `StellaOps.Workflow.Abstractions` or `Ablera.Serdica.Workflow.Abstractions`**. Absent those, it is a no-op. No opt-in attribute is needed anywhere. + +## Scope + +The analyzer inspects the `Spec` property of every `IDeclarativeWorkflow`-implementing class in the compilation. From each `Spec` initializer it walks transitively into every method call whose target is available as source: + +- **Trusted leaves** — any method/type in an assembly whose name starts with `StellaOps.Workflow.` or `Ablera.Serdica.Workflow.`. Not walked; always allowed. +- **Same compilation / project references** — source walked via `DeclaringSyntaxReferences`. Violations in those methods are reported at their native location (same compilation) or surfaced as **WF010** at the workflow call site with an additional location inside the helper (cross-project). +- **Metadata-only (compiled NuGet / framework) references** — not walkable. Invocations, object creations, and non-const field/property reads on such assemblies are **rejected**. If you need a helper routine, put it in a project reference, not a binary dependency. + +## Rules + +All diagnostics are `Error` severity. They fail `dotnet build`. + +### WF001 — Imperative control flow {#wf001} + +**Bad** + +```csharp +public WorkflowSpec Spec { get; } = Build(); +private static WorkflowSpec Build() +{ + if (DateTime.UtcNow.Hour > 12) + { + return WorkflowSpec.For().InitializeState(...).Build(); + } + return WorkflowSpec.For().InitializeState(...).Build(); +} +``` + +**Why it fails** — The shape of the canonical JSON would depend on when the workflow class was constructed. Two callers building the same workflow get different definitions. + +**Fix** — Express branching inside the canonical model: + +```csharp +public WorkflowSpec Spec { get; } = WorkflowSpec.For() + .StartWith(flow => flow + .WhenExpression( + "After noon?", + WorkflowExpr.Gt(WorkflowExpr.Path("start.hour"), WorkflowExpr.Number(12)), + whenTrue => whenTrue.Call(...), + whenElse => whenElse.Call(...))) + .Build(); +``` + +Forbidden constructs: `if`, `for`, `foreach`, `while`, `do`, `switch` (statement form), `try` / `catch` / `finally`, `throw`, `lock`, `using`, `goto`, `break`, `continue`, `yield`. + +### WF002 — Async/await {#wf002} + +**Bad** + +```csharp +public WorkflowSpec Spec { get; } = BuildAsync().GetAwaiter().GetResult(); +private static async Task> BuildAsync() +{ + await Task.Delay(1); + return WorkflowSpec.For().Build(); +} +``` + +**Why it fails** — The workflow Spec is constructed once, synchronously. `await` / `async` / `Task.Run` / `Thread` introduce runtime concurrency primitives that cannot be represented in the canonical definition. + +**Fix** — Remove the async machinery. If the work you were doing was async only because it read from a DB or an API, move it out of the Spec: those reads should happen inside workflow steps at runtime (via `builder.Call(...)`), not at definition-construction time. + +### WF003 — Call into non-trusted assembly {#wf003} + +**Bad** + +```csharp +public WorkflowSpec Spec { get; } = WorkflowSpec.For() + .InitializeState(WorkflowExpr.Obj( + WorkflowExpr.Prop("machine", WorkflowExpr.String(Environment.MachineName)))) + .Build(); +``` + +**Why it fails** — `Environment.MachineName` is a property read on `System.Environment` (in `System.Runtime`, a metadata-only reference). The canonical JSON would bake whatever machine built the workflow. + +**Fix** — For runtime data, bind through a path reference: `WorkflowExpr.Path("start.machineName")`. For literals, use the typed builders: `WorkflowExpr.String("fixed-value")`. For helper routines, put them in a project-referenced class so the analyzer can walk them. + +Lambda invocations via `func()` (where `func` is an `Action`/`Func`/custom delegate) are **not** flagged as WF003 — the lambda body is walked inline. `nameof(...)` expressions are also skipped. + +### WF004 — `new` on non-trusted type {#wf004} + +**Bad** + +```csharp +public WorkflowSpec Spec { get; } = WorkflowSpec.For() + .InitializeState(WorkflowExpr.Obj( + WorkflowExpr.Prop("id", WorkflowExpr.String(new Guid().ToString())))) + .Build(); +``` + +**Why it fails** — `new Guid()` instantiates a BCL type at workflow-build time; every class construction produces a different Guid, and the canonical JSON is no longer deterministic. + +**Fix** — Use a workflow expression. Workflow DTOs (`LegacyRabbitAddress`, `WorkflowBusinessReferenceDeclaration`, `WorkflowNamedExpressionDefinition`, etc.) that live in `StellaOps.Workflow.*` / `Ablera.Serdica.Workflow.*` are trusted and may be instantiated freely. + +### WF005 — C# conditional operator in workflow code {#wf005} + +**Bad** + +```csharp +WorkflowExpr.Prop("route", WorkflowExpr.String(useFastLane ? "fast" : "slow")) +WorkflowExpr.Prop("name", WorkflowExpr.String(input.Name ?? "anonymous")) +WorkflowExpr.Prop("length", WorkflowExpr.Number(input?.Name?.Length ?? 0)) +``` + +**Why it fails** — `?:`, `??`, and `?.` resolve at workflow-**build** time. Whichever branch wins when the Spec is constructed is what ends up in the canonical JSON; the runtime condition never enters the workflow at all. + +**Fix** — Use canonical equivalents: + +```csharp +WorkflowExpr.Prop("route", WorkflowExpr.Func("if", + WorkflowExpr.Path("state.useFastLane"), + WorkflowExpr.String("fast"), + WorkflowExpr.String("slow"))) + +WorkflowExpr.Prop("name", WorkflowExpr.Func("coalesce", + WorkflowExpr.Path("start.name"), + WorkflowExpr.String("anonymous"))) +``` + +### WF006 — Non-trusted field/property read {#wf006} + +**Bad** + +```csharp +WorkflowExpr.Prop("year", WorkflowExpr.Number(DateTime.UtcNow.Year)) +WorkflowExpr.Prop("cfg", WorkflowExpr.String(MyStaticConfig.CurrentRegion)) +``` + +**Why it fails** — Reading a non-const field or property on a type that isn't in a trusted or source-walkable assembly bakes the read value into the canonical JSON. `DateTime.UtcNow.Year` gives you the year-at-build, not a per-execution value. `MyStaticConfig.CurrentRegion` couples the workflow definition to whatever config was loaded when the plugin assembly initialized. + +**What is allowed** + +- **Compile-time constants** (`const` fields) on any type. `int.MaxValue` and user `public const string Route = "/x"` are always fine. +- **Static readonly fields** on types in the current compilation or a project reference — the analyzer can walk the initializer and verify it's canonical. This is how `static readonly LegacyRabbitAddress QueryAddress = new("pas_query");` stays legal. +- Any member on a `StellaOps.Workflow.*` / `Ablera.Serdica.Workflow.*` type. + +**Fix** — Use `WorkflowExpr.Path(...)` to bind a runtime value, or a `const` on your own class for compile-time constants. + +### WF010 — Reachable helper violation {#wf010} + +Emitted when a helper method reachable from a workflow's `Spec` (in a project reference) itself contains a WF001–WF006 violation. The primary location points at the call site inside the workflow; the additional location points at the offending line inside the helper. + +If the helper is in the **same compilation** as the workflow, the underlying rule (e.g., WF001) fires directly at the helper's line instead. WF010 is specifically for cross-project indirection where the analyzer cannot emit diagnostics into the referenced project's source. + +**Fix** — Fix the helper. One fix clears the diagnostic at every call site. + +## Non-goals for v1 + +- **No source generator.** The analyzer only validates; the runtime `WorkflowCanonicalDefinitionCompiler` remains the single source of truth for canonical JSON emission. Adding a generator would create two sources of truth that will drift. +- **No severity configuration.** All rules are `Error`. If a workflow would fail canonicalization at runtime, building it should fail too. +- **No escape hatch attribute.** If a real case forces one (e.g., a vetted BCL helper we want to whitelist) it can be added later. Shipping it from day one invites misuse. + +## When the analyzer will NOT catch something + +- **Reflection** (`Type.GetMethod(...)`, `Activator.CreateInstance(...)`): flagged as WF003 (invocation of metadata-only `System.Reflection`). +- **Runtime-configured workflows built by a service/DI**: the `Spec` property's initializer is the entry point. If the workflow class has a constructor that fetches data before building `Spec`, the analyzer only walks the `Spec` getter — the constructor runs at runtime and is outside scope. Keep workflow definitions pure (no constructor state dependency). + +## Quick reference + +| ID | Short name | Severity | +|----|------------|----------| +| WF001 | Imperative control flow | Error | +| WF002 | Async/await | Error | +| WF003 | Call into non-trusted assembly | Error | +| WF004 | `new` on non-trusted type | Error | +| WF005 | C# `?:` / `??` / `?.` | Error | +| WF006 | Non-trusted field/property read | Error | +| WF010 | Reachable helper violation (cross-project) | Error | diff --git a/src/Workflow/__Tests/StellaOps.Workflow.Analyzer.Tests/GoldenWorkflowShapeTests.cs b/src/Workflow/__Tests/StellaOps.Workflow.Analyzer.Tests/GoldenWorkflowShapeTests.cs new file mode 100644 index 000000000..fbabca4b2 --- /dev/null +++ b/src/Workflow/__Tests/StellaOps.Workflow.Analyzer.Tests/GoldenWorkflowShapeTests.cs @@ -0,0 +1,176 @@ +using FluentAssertions; +using NUnit.Framework; + +namespace StellaOps.Workflow.Analyzer.Tests; + +/// +/// Regression tests that exercise the patterns used by the real Serdica workflow corpus. +/// A failure here means the analyzer has regressed and would reject canonical code that +/// was previously accepted. +/// +[TestFixture] +public class GoldenWorkflowShapeTests +{ + [Test] + public async Task RealShape_LegacyRabbitAddressFieldsPlusWhenExpressionPlusCall_IsClean() + { + const string source = """ + using System; + using System.Collections.Generic; + using StellaOps.Workflow.Abstractions; + using StellaOps.Workflow.Contracts; + + public sealed class VehicleClaimRequest { public string? SrPolicyId { get; set; } } + + public sealed class VehicleClaim : IDeclarativeWorkflow + { + private const string EligibilityTaskName = "Eligibility Check"; + private const string PaymentTaskName = "Payment"; + private static readonly LegacyRabbitAddress ClaimsQueryAddress = new("pas_claims_cclaims_get"); + private static readonly LegacyRabbitAddress ConvertToClaimAddress = new("pas_claimprocessing_setclaimsubstatus"); + + public string WorkflowName => "VehicleClaim"; + public string WorkflowVersion => "1.0.0"; + public string DisplayName => "Vehicle Claim"; + public IReadOnlyCollection WorkflowRoles => new string[0]; + public IReadOnlyCollection Tasks => new WorkflowTaskDescriptor[0]; + + public WorkflowSpec Spec { get; } = WorkflowSpec.For() + .InitializeState(WorkflowExpr.Obj( + WorkflowExpr.Prop("lob", WorkflowExpr.String("MOTOR")), + WorkflowExpr.Prop("productCode", WorkflowExpr.Func( + "coalesce", + WorkflowExpr.Path("start.productCode"), + WorkflowExpr.String("4704"))), + WorkflowExpr.Prop("srPolicyId", WorkflowExpr.Path("start.srPolicyId")), + WorkflowExpr.Prop("notifiedAt", WorkflowExpr.Null()))) + .StartWith(flow => flow + .Call( + "Query Claims", + ClaimsQueryAddress, + WorkflowExpr.Obj( + WorkflowExpr.Prop("SrPolicyId", WorkflowExpr.Path("state.srPolicyId"))), + resultKey: "claimsList") + .WhenExpression( + "Has prior claim?", + WorkflowExpr.Gt( + WorkflowExpr.Func("length", WorkflowExpr.Path("result.claimsList")), + WorkflowExpr.Number(0)), + whenTrue => whenTrue.Call( + "Convert To Claim", + ConvertToClaimAddress, + WorkflowExpr.Obj( + WorkflowExpr.Prop("SrPolicyId", WorkflowExpr.Path("state.srPolicyId")))), + whenElse => whenElse.ActivateTask(EligibilityTaskName))) + .AddTask(WorkflowHumanTask.For( + EligibilityTaskName, "VehicleClaimEligibility", "default") + .WithRoles("DBA", "APR") + .WithPayload(WorkflowExpr.Obj( + WorkflowExpr.Prop("srPolicyId", WorkflowExpr.Path("state.srPolicyId")))) + .OnComplete(flow => flow + .Set("eligibilityAnswer", WorkflowExpr.Path("payload.answer")) + .WhenExpression( + "Approved?", + WorkflowExpr.Eq( + WorkflowExpr.Path("state.eligibilityAnswer"), + WorkflowExpr.String("approve")), + whenTrue => whenTrue.ActivateTask(PaymentTaskName), + whenElse => whenElse.Complete()))) + .AddTask(WorkflowHumanTask.For( + PaymentTaskName, "VehicleClaimPayment", "default") + .WithRoles("PAY") + .OnComplete(flow => flow.Complete())) + .Build(); + } + """; + + var diagnostics = await AnalyzerTestHarness.RunAsync(source); + var workflowDiagnostics = diagnostics.Where(d => d.Id.StartsWith("WF")).ToArray(); + workflowDiagnostics.Should().BeEmpty( + "a workflow built entirely from fluent builder calls + WorkflowExpr + trusted address " + + "literals + SetBusinessReference + WhenExpression + OnComplete must not produce any WF* diagnostic"); + } + + [Test] + public async Task RealShape_SetBusinessReferenceWithNestedDeclaration_IsClean() + { + const string source = """ + using System; + using System.Collections.Generic; + using StellaOps.Workflow.Abstractions; + using StellaOps.Workflow.Contracts; + + public sealed class AssistantAddAnnexRequest + { + public string? SrPolicyId { get; set; } + public string? SrAnnexId { get; set; } + } + + public sealed class AssistantAddAnnex : IDeclarativeWorkflow + { + public string WorkflowName => "AssistantAddAnnex"; + public string WorkflowVersion => "1.0.0"; + public string DisplayName => "Add Annex"; + public IReadOnlyCollection WorkflowRoles => new string[0]; + public IReadOnlyCollection Tasks => new WorkflowTaskDescriptor[0]; + + public WorkflowSpec Spec { get; } = WorkflowSpec.For() + .InitializeState(WorkflowExpr.Obj( + WorkflowExpr.Prop("srPolicyId", WorkflowExpr.Path("start.srPolicyId")), + WorkflowExpr.Prop("srAnnexId", WorkflowExpr.Path("start.srAnnexId")))) + .StartWith(flow => flow + .SetBusinessReference(new WorkflowBusinessReferenceDeclaration + { + KeyExpression = WorkflowExpr.Path("start.srPolicyId"), + Parts = new WorkflowNamedExpressionDefinition[] + { + new WorkflowNamedExpressionDefinition { Name = "policyId", Expression = WorkflowExpr.Path("start.srPolicyId") }, + new WorkflowNamedExpressionDefinition { Name = "annexId", Expression = WorkflowExpr.Path("start.srAnnexId") }, + }, + }) + .Complete()) + .Build(); + } + """; + + var diagnostics = await AnalyzerTestHarness.RunAsync(source); + var workflowDiagnostics = diagnostics.Where(d => d.Id.StartsWith("WF")).ToArray(); + workflowDiagnostics.Should().BeEmpty( + "SetBusinessReference(new WorkflowBusinessReferenceDeclaration { ... }) is the canonical pattern " + + "for attaching business keys; all types are in the trusted assembly prefix"); + } + + [Test] + public async Task RealShape_CustomFunctionCalls_CustomBulstradFunction_IsClean() + { + const string source = """ + using System; + using System.Collections.Generic; + using StellaOps.Workflow.Abstractions; + using StellaOps.Workflow.Contracts; + + public sealed class CustomerChangeRequest { public string? Customer { get; set; } } + + public sealed class CustomerChange : IDeclarativeWorkflow + { + public string WorkflowName => "CustomerChange"; + public string WorkflowVersion => "1.0.0"; + public string DisplayName => "Customer Change"; + public IReadOnlyCollection WorkflowRoles => new string[0]; + public IReadOnlyCollection Tasks => new WorkflowTaskDescriptor[0]; + + public WorkflowSpec Spec { get; } = WorkflowSpec.For() + .InitializeState(WorkflowExpr.Obj( + WorkflowExpr.Prop("customer", WorkflowExpr.Func( + "bulstrad.normalizeCustomer", + WorkflowExpr.Path("start.customer"))))) + .Build(); + } + """; + + var diagnostics = await AnalyzerTestHarness.RunAsync(source); + var workflowDiagnostics = diagnostics.Where(d => d.Id.StartsWith("WF")).ToArray(); + workflowDiagnostics.Should().BeEmpty( + "custom WorkflowExpr.Func(\"bulstrad.*\", ...) dispatches at runtime; the function name is a string literal"); + } +}