Implement per-orchestration and per-activity versioning#695
Draft
Implement per-orchestration and per-activity versioning#695
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Split into two focused samples: - WorkerVersioningSample: deployment-based versioning with UseDefaultVersion() - PerOrchestratorVersioningSample: multi-version routing with [DurableTaskVersion] Both tested against the DTS emulator. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The v1 orchestrator uses an AlreadyMigrated flag in the input to prevent infinite ContinueAsNew loops if the backend does not propagate NewVersion. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Imported src/Grpc/orchestrator_service.proto from microsoft/durabletask-protobuf branch torosent/activity-request-tags (PR #68) instead of carrying the contract change as a local hand edit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Refresh src/Grpc/orchestrator_service.proto from durabletask-protobuf main after PR #68 merged and update src/Grpc/versions.txt to the mainline source commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolves conflicts in: - src/Grpc/versions.txt: take main's newer protobuf import - test/Worker/Core.Tests/Shims/TaskOrchestrationContextWrapperTests.cs: union activity-versioning tests with the new preserveUnprocessedEvents test - src/Worker/Core/DependencyInjection/DurableTaskWorkerWorkItemFiltersValidator.cs: semantic conflict — main's validator referenced TaskName-keyed registry dicts while this branch keys them by OrchestratorVersionKey/ActivityVersionKey. Updated FindUnknown to use a HashSet of registered logical names so the validator works regardless of how many versions are registered for a name. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…combined Addresses gap (4) in proposal #692: per-task [DurableTaskVersion] routing and worker-level UseVersioning() both consume the orchestration instance version field. When combined with MatchStrategy != None the worker-level filter runs before per-task dispatch and silently rejects orchestrations whose instance version does not match the worker version, masking per-task routing. This change emits a single Warning-level log entry (EventId 78) at worker construction when both features are configured, pointing the operator at the docs and clarifying that the two features are not designed to be combined. Implementation notes: - Added DurableTaskRegistry.HasAnyVersionedRegistration() extension in Worker.csproj (which already has IVT to Abstractions internals). - Threaded an optional IOptionsMonitor<DurableTaskRegistry> through the GrpcDurableTaskWorker constructor so the check can run at startup without coupling it to the work-item filter path. - Added three unit tests covering: combined-strict (warns), per-task only (no warn), worker-level only (no warn). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+31
to
+37
| foreach (OrchestratorVersionKey key in registry.Orchestrators.Keys) | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(key.Version)) | ||
| { | ||
| return true; | ||
| } | ||
| } |
Comment on lines
+39
to
+45
| foreach (ActivityVersionKey key in registry.Activities.Keys) | ||
| { | ||
| if (!string.IsNullOrWhiteSpace(key.Version)) | ||
| { | ||
| return true; | ||
| } | ||
| } |
…validation, helper-conflict throw, line endings Closes review findings #1, #5, #6, #8 from PR #695: - ToVersionSuffix collision (#1): the previous encoder left underscore unescaped, so '1.0' and '1_x002E_0' both produced '_1_x002E_0'. Now every non-alphanumeric character (including '_') is escaped as '_xHHHH_', making the encoding injective and avoiding silent generated-helper collisions. - Whitespace version validation (#5): DurableTaskVersionAttribute and the versioned AddOrchestrator/AddActivity overloads now reject whitespace-only version strings (null/empty still means 'unversioned'). The source generator emits a new error diagnostic DURABLE3005 so the misconfiguration is caught at compile time rather than first instance construction. - Helper-conflict throw (#8): generated helpers ApplyGeneratedVersion / ApplyGeneratedActivityVersion now throw InvalidOperationException at runtime when a caller-supplied options.Version disagrees with the version baked into the generator-emitted helper name. Previously the caller's version silently won, defeating the purpose of the version-suffixed method. Updated the generator-output snapshot tests accordingly. - TaskOptions.cs CRLF/LF normalization (#6): the file was the only one in the repo with mixed/CRLF line endings, polluting the PR diff. Normalized to LF to match the rest of the codebase. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…it-unversioned activity, fail-closed tag, throw on combined versioning Closes review findings #2, #3, #4, #7 from PR #695: - Orchestrator fallback strictness (#2): DurableTaskFactory.TryCreateOrchestrator no longer falls back to the unversioned registration when at least one versioned registration exists for the same logical name. Previously, scheduling 'PaymentWorkflow' v3 against a registry that had v1, v2, and an unversioned default would silently route the call to the unversioned default. Now this returns 'not found' so the caller is told what actually happened. The unversioned-fallback path is preserved when no versioned registration exists for the name (the migration scenario where pre-versioning instances scheduled with a specific version still need to dispatch). Same gating applied to TryCreateActivity for symmetry on the inherited-fallback path. - Explicit-unversioned activity selection (#3): the API now distinguishes 'inherit' from 'explicit unversioned'. ActivityOptions.Version = null still means inherit; ActivityOptions.Version = TaskVersion.Unversioned (or any non-null TaskVersion, including default) is treated as an explicit selection — even when the version string is empty. This lets a v2 orchestration call the unversioned activity by passing TaskVersion.Unversioned. Added the public TaskVersion.Unversioned static for clarity. - Tag fail-closed (#4): the dispatch-routing tag is renamed from 'microsoft.durabletask.activity.explicit-version' (boolean) to 'microsoft.durabletask.activity.version-source' with values 'explicit' or 'inherited'. The SDK now stamps the tag for both cases (not just explicit), and the worker treats a missing tag on a versioned request as 'explicit' — i.e., strict, no fallback. This means an older sidecar that drops tags can no longer silently degrade strict-explicit semantics to inherited-with-fallback; instead, those calls fail closed with 'activity not found'. - Combined-versioning guard escalated and centralized (#7): the warning-only check on combined UseVersioning() + [DurableTaskVersion] is now a fail-fast InvalidOperationException at worker construction. Moved into a shared WorkerVersioningPolicy.EnsureNotCombined helper in Worker.csproj so any worker subclass (gRPC and any future transport) gets the same check. EventId 78 warning log entry removed. Tests updated to reflect new semantics: - TryCreateOrchestrator_WithMixedRegistrations_DoesNotFallBackForUnknownVersion (replaces old 'UsesUnversionedFallbackForUnknownVersion'). - TryCreateOrchestrator_WithOnlyUnversionedRegistration_FallsBackForVersionedRequest (preserves the migration path). - CallActivityAsync_ExplicitUnversionedActivityOption_BypassesInherit (new b2 coverage). - CallActivityAsync tag assertions updated to the new VersionSourceTagName / ExplicitSource / InheritedSource constants, including a new test that an unversioned-orchestration unversioned-activity stamps no tag at all. - Constructor_PerTaskVersioningCombinedWithStrictWorkerVersioning_Throws (replaces old 'LogsWarning'). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+261
to
+282
| foreach (AttributeData attributeData in classType.GetAttributes()) | ||
| { | ||
| if (attributeData.AttributeClass?.ToDisplayString() == "Microsoft.DurableTask.DurableTaskVersionAttribute" | ||
| && attributeData.ConstructorArguments.Length > 0 | ||
| && attributeData.ConstructorArguments[0].Value is string version) | ||
| { | ||
| if (version.Length > 0 && string.IsNullOrWhiteSpace(version)) | ||
| { | ||
| hasWhitespaceVersion = true; | ||
| taskVersionLocation = attributeData.ApplicationSyntaxReference?.GetSyntax().GetLocation(); | ||
| // Treat as unversioned for downstream emission so we don't generate code referencing | ||
| // a whitespace literal; the diagnostic below will fail the build. | ||
| taskVersion = string.Empty; | ||
| } | ||
| else | ||
| { | ||
| taskVersion = version; | ||
| } | ||
|
|
||
| break; | ||
| } | ||
| } |
Comment on lines
+409
to
+419
| foreach (DurableTaskTypeInfo task in allTasks) | ||
| { | ||
| if (task.HasWhitespaceVersion) | ||
| { | ||
| Location location = task.TaskVersionLocation ?? task.TaskNameLocation ?? Location.None; | ||
| context.ReportDiagnostic(Diagnostic.Create( | ||
| WhitespaceTaskVersionRule, | ||
| location, | ||
| task.TaskName)); | ||
| } | ||
| } |
Comment on lines
+164
to
+171
| foreach (string reserved in ActivityVersioning.ReservedTagKeys) | ||
| { | ||
| if (string.Equals(key, reserved, StringComparison.Ordinal)) | ||
| { | ||
| isReserved = true; | ||
| break; | ||
| } | ||
| } |
…flict, TaskVersion null-safety, integration test refresh, migration recipe Closes the second-pass review findings from PR #695: #1 — Stale tag references in integration tests: VersionedClassSyntaxTestOrchestration.cs and VersionedClassSyntaxIntegrationTests.cs were still using the pre-rename 'microsoft.durabletask.activity.explicit-version' constant. The spoof-protection test in particular was passing for the wrong reason since the SDK no longer recognizes that key. Updated both files to use ActivityVersioning.VersionSourceTagName / ExplicitSource / InheritedSource and added IVT for Worker.Grpc.IntegrationTests in Worker.csproj so the integration tests can reference the SDK constants directly. #2 — Generator helper conflict check missed explicit-unversioned: ApplyGeneratedVersion (StartOrchestrationOptions / SubOrchestrationOptions) and ApplyGeneratedActivityVersion all used patterns that required '.Version.Length > 0' or '!IsNullOrWhiteSpace', which excluded TaskVersion.Unversioned. A user calling CallProcessPayment_2Async(..., new ActivityOptions { Version = TaskVersion.Unversioned }) silently got v2 instead of the contradicting throw the helper was supposed to enforce. Now the patterns match any non-null Version (including the empty/unversioned case) and the diagnostic message distinguishes 'explicit-unversioned' from a specific version. Generator-output snapshot tests updated. #3 — TaskVersion null-storage caused null/empty mismatch and GetHashCode crash: the constructor stored 'null' verbatim when given null. Effects: TaskVersion.Unversioned.Equals(new TaskVersion('')) was false, and TaskVersion.Unversioned.GetHashCode() called StringComparer.OrdinalIgnoreCase.GetHashCode(null) which throws — making any user who put TaskVersion in a dictionary key crash at runtime. Constructor now normalizes null/empty to string.Empty; Equals and GetHashCode are null-safe and treat null and empty as identical. (default(TaskVersion) still has Version=null at the field level — that's a struct-default constraint — but Equals/GetHashCode handle it.) #4 — Whitespace TaskVersion ctor validation: the registry rejected whitespace-only TaskVersion, but new TaskVersion(' ') itself accepted the value, so it could leak into ActivityOptions / StartOrchestrationOptions / SubOrchestrationOptions and route silently to 'no exact match'. The constructor now throws ArgumentException for non-empty whitespace. The redundant ValidateRegistrationVersion method and the explicit whitespace check in DurableTaskVersionAttribute were removed because TaskVersion's constructor handles them centrally. #7 — Migration recipe: added a new section to the proposal documenting the 'add [DurableTaskVersion] to an existing class' migration path. Recommended recipe is keep the unversioned class registered until in-flight unversioned instances drain or ContinueAsNew to the new version. Reverse migration (removing [DurableTaskVersion]) is documented as unsupported. #8 — Tag rename pre-release note: added a one-paragraph note in the proposal acknowledging the in-flight rename from 'explicit-version' (boolean) to 'version-source' ('explicit'/'inherited') and recommending pre-release deployments drain before pointing at the new contract since the worker now fail-closes on missing tag for versioned activity requests. Tests: 84 generator + 127 worker.tests + 135 worker.grpc.tests + 159 abstractions.tests + 8 integration = 513 total, all passing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What changed?
ActivityOptions.Versionoverrides, inherited version fallback behavior,ContinueAsNewversion migration, and strict worker-version filter handling.docs/superpowersplanning/spec artifacts from the branch diff.Why is this change needed?
#692to a working end-to-end implementation with generator, runtime, sample, and test coverage.Issues / work items
Project checklist
release_notes.mdAI-assisted code disclosure (required)
Was an AI tool used? (select one)
If AI was used:
src/Abstractions/*src/Worker/Core/*src/Worker/Grpc/*src/InProcessTestHost/*src/Grpc/orchestrator_service.prototest/Worker/Core.Tests/*test/Grpc.IntegrationTests/*samples/*versioning*AI verification (required if AI was used):
Testing
Automated tests
dotnet build Microsoft.DurableTask.sln --nologo --verbosity minimaldotnet test test/Worker/Core.Tests/Worker.Tests.csproj --nologo --verbosity minimaldotnet test test/Worker/Grpc.Tests/Worker.Grpc.Tests.csproj --nologo --verbosity minimaldotnet test test/Grpc.IntegrationTests/Grpc.IntegrationTests.csproj --filter "FullyQualifiedName~VersionedClassSyntaxIntegrationTests" --nologo --verbosity minimalManual validation (only if runtime/behavior changed)
VersionedClassSyntaxIntegrationTestsNotes for reviewers
src/Grpc/orchestrator_service.protois sourced frommicrosoft/durabletask-protobuf; aftermicrosoft/durabletask-protobuf#68merged,src/Grpc/versions.txtwas refreshed to the currentmaincommit.mainalso pulls in the additiveRewindOrchestrationActioncontract already present in the protobuf source of truth.ActivityRequest.tags; sidecars outside this repo need matching support to preserve the same semantics end-to-end.TaskScheduledEvent.Tagsin the in-proc protobuf path.