Implement null TenantId for tenant-agnostic entities

[sfmskywalker]

Implement null TenantId for tenant-agnostic entities

This change introduces the use of null as a TenantId to represent tenant-agnostic entities, making them visible across all tenants. It updates EF Core query filters and the ActivityRegistry to include tenant-agnostic entities based on the new convention, and updates the Elsa.sln file with the new ADR.

---

Changes

• ADR-0009
  Added a new Architectural Decision Record (ADR-0009) defining the use of null for tenant-agnostic entities and its implications.

• EF Core Query Filter
  Modified SetTenantIdFilter to include records where TenantId is null, in addition to matching the current tenant ID.

• Activity Registry
  Updated ActivityRegistry query methods to include activity descriptors with a null TenantId.

• ActivityDescriptor
  Added a nullable TenantId property to the ActivityDescriptor model.

• WorkflowDefinitionActivityDescriptorFactory
  Modified to allow null TenantId values on ActivityDescriptor instances.

• ClrWorkflowsProvider
  Removed tenant prefix logic, relying solely on the TenantId property.

• Test Fixtures
  Added ITenantAccessor to ActivityTestFixture.

• Component Tests
  Added a test tenant resolver and configured tenant scope.

• Configuration
  Enabled multitenancy in Elsa.Server.Web/Program.cs.

---

Impact

• Behavioral Changes
  Entities with a null TenantId are now accessible across all tenants. EF Core queries and ActivityRegistry methods will return tenant-agnostic entities.

• Dependencies Affected
  Elsa.Persistence.EFCore.Common and Elsa.Workflows.Core are directly affected due to changes in tenant ID filtering and activity registration.

• Nullable Handling
  Code paths must correctly handle nullable TenantId fields.

[Copilot]

Pull request overview

Adds tenant-aware behavior to testing infrastructure and core registries so tests (and runtime components) can operate in a tenant-agnostic way, while also introducing an ADR describing null tenant IDs as “tenant-agnostic”.

Changes:

• Registers ITenantAccessor in ActivityTestFixture and sets a default tenant context for component tests.
• Introduces tenant-aware ActivityDescriptor/ActivityRegistry behavior and updates workflow-definition activity descriptors to carry a TenantId.
• Updates EF Core tenant query filter behavior to include TenantId == null entities and adds ADR-0009 documenting the null tenant ID convention.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/Workflows/DeleteWorkflow.cs	Removes hard-coded tenant assignment for test workflow definition.
test/component/Elsa.Workflows.ComponentTests/Scenarios/WorkflowActivities/DeleteWorkflowTests.cs	Adds verification that the deleted workflow definition no longer exists in the store.
test/component/Elsa.Workflows.ComponentTests/Helpers/Services/ComponentTestTenantResolver.cs	Adds a component-test tenant resolver that resolves to the default tenant.
test/component/Elsa.Workflows.ComponentTests/Helpers/Fixtures/WorkflowServer.cs	Configures multitenancy/tenant resolver pipeline for component test server.
test/component/Elsa.Workflows.ComponentTests/Helpers/Abstractions/AppComponentTest.cs	Pushes a default tenant context into the test scope.
src/modules/Elsa.Workflows.Runtime/Providers/ClrWorkflowsProvider.cs	Removes tenant-prefixed IDs and tenant accessor dependency when materializing CLR workflows.
src/modules/Elsa.Workflows.Management/Activities/WorkflowDefinitionActivity/WorkflowDefinitionActivityDescriptorFactory.cs	Stops tenant-prefixing type names and instead stores tenant ID on the descriptor.
src/modules/Elsa.Workflows.Core/Services/ActivityRegistry.cs	Introduces tenant-aware indexing/filtering for activity descriptors.
src/modules/Elsa.Workflows.Core/Models/ActivityDescriptor.cs	Adds nullable TenantId to represent tenant-agnostic descriptors.
src/modules/Elsa.Persistence.EFCore.Common/EntityHandlers/SetTenantIdFilter.cs	Expands EF Core query filter to include TenantId == null entities.
src/modules/Elsa.Common/Multitenancy/Implementations/DefaultTenantAccessor.cs	Adds TenantId convenience property to the tenant accessor.
src/modules/Elsa.Common/Multitenancy/Contracts/ITenantAccessor.cs	Adds TenantId to the tenant accessor contract.
src/common/Elsa.Testing.Shared/ActivityTestFixture.cs	Registers ITenantAccessor for activity unit testing fixture.
src/apps/Elsa.Server.Web/Program.cs	Enables multitenancy in the sample server app by default.
doc/adr/toc.md	Adds ADR-0009 to ADR table of contents.
doc/adr/graph.dot	Adds ADR-0009 node to ADR graph.
doc/adr/0009-null-tenant-id-for-tenant-agnostic-entities.md	Documents the new null tenant ID convention for tenant-agnostic entities.
Elsa.sln	Adds ADR-0009 to the solution items.

[Copilot]

@sfmskywalker I've opened a new pull request, #7227, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@sfmskywalker I've opened a new pull request, #7228, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@sfmskywalker I've opened a new pull request, #7229, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@sfmskywalker I've opened a new pull request, #7230, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

@sfmskywalker I've opened a new pull request, #7231, to work on those changes. Once the pull request is ready, I'll request review from you.

[Copilot]

Pull request overview

Copilot reviewed 31 out of 35 changed files in this pull request and generated 8 comments.

Files not reviewed (1)

• src/modules/Elsa.Persistence.EFCore.SqlServer/Migrations/Management/20260131023442_ConvertNullTenantIdToEmptyString.Designer.cs: Language not supported

[Copilot]

ActivityDescriptor.TenantId is documented as “Null means tenant-agnostic”, but ADR-0009 establishes "" as the explicit tenant-agnostic marker for persisted entities and describes null as transient/unassigned. Consider updating this comment to clarify the distinction (e.g., null allowed for in-memory descriptors, "" for explicit tenant-agnostic scope) to avoid confusion for contributors and consumers.

Suggested change

	public string? TenantId { get; set; } // Null means tenant-agnostic.
	// Per ADR-0009, "*" is the explicit tenant-agnostic marker for persisted entities.
	// A null value indicates a transient or in-memory descriptor with no tenant assigned yet.
	public string? TenantId { get; set; }

[Copilot]

This ADR (and the implementation) establishes "*" as the tenant-agnostic marker and treats null as unassigned, but the PR title/description says tenant-agnostic is represented by TenantId = null. Please align the PR description/title with the actual convention, or adjust the implementation if null is still intended to be the persisted marker.

[Copilot]

These tests cover the "*" sentinel, but ActivityRegistry also treats TenantId = null as tenant-agnostic (GetOrCreateRegistry checks for null). Add a test case with tenantId=null to ensure descriptors produced without an explicit tenant ID (e.g., built-in activities) resolve correctly.

[Copilot]

TenantAwareDbContextFactory is assigning ElsaDbContextBase.TenantId from tenantAccessor.Tenant?.Id, which can be null when no tenant context is pushed. With the updated conventions (NormalizeTenantId => null becomes ""), this can cause the EF query filter to compare against NULL and unintentionally exclude default-tenant rows (TenantId="") while only matching TenantId IS NULL or "*". Use tenantAccessor.TenantId (normalized) when setting ElsaDbContextBase.TenantId so the default tenant is consistently "".

[Copilot]

SaveManyAsync pulls the tenant ID from ITenantAccessor.Tenant?.Id, which can be null when no tenant context is pushed. With ADR-0008 normalizing the default tenant to "", this can result in entities being bulk-upserted with TenantId still null (and subsequently not matched by the query filter). Consider using ITenantAccessor.TenantId (normalized) here.

Suggested change

	var tenantId = serviceProvider.GetRequiredService<ITenantAccessor>().Tenant?.Id;
	var tenantId = serviceProvider.GetRequiredService<ITenantAccessor>().TenantId;

[Copilot]

This assignment is guarded by tenantId != null, so when the current tenant is the default (""), entities with TenantId == null won't be fixed up and may be saved as NULL. If the intent is “null = unassigned”, you likely want to assign even when the tenant ID is the default empty string.

Suggested change

	if (entityWithTenant.TenantId == null && tenantId != null)
	if (entityWithTenant.TenantId == null)

[Copilot]

If the provider returns 0 descriptors, descriptorsByTenant will be empty and the method will never clear any previously registered descriptors for this provider, leaving stale activity descriptors after deletions. Handle the empty-descriptor case by clearing existing descriptors for this provider (at least for the current tenant and the agnostic registry) before returning.

Suggested change

	// If the provider returns no descriptors, clear any previously registered descriptors
	// for this provider from the current tenant and the tenant-agnostic registry.
	if (!descriptors.Any())
	{
	// Normalize the current tenant ID for grouping.
	var currentTenantKey = NormalizeTenantIdForGrouping(tenantAccessor.TenantId);
	var agnosticTenantKey = NormalizeTenantIdForGrouping(null);
	// Local function to clear all descriptors for this provider from a given registry.
	void ClearProviderFromRegistry(string tenantKey)
	{
	var registry = GetOrCreateRegistry(tenantKey);
	if (registry.ProvidedActivityDescriptors.TryGetValue(providerType, out var oldDescriptors))
	{
	foreach (var oldDescriptor in oldDescriptors.ToList())
	{
	registry.ActivityDescriptors.TryRemove((oldDescriptor.TypeName, oldDescriptor.Version), out _);
	}
	registry.ProvidedActivityDescriptors.TryRemove(providerType, out _);
	}
	}
	// Clear descriptors for the current tenant.
	ClearProviderFromRegistry(currentTenantKey);
	// Clear descriptors for the tenant-agnostic registry, if different.
	if (!string.Equals(currentTenantKey, agnosticTenantKey, StringComparison.Ordinal))
	ClearProviderFromRegistry(agnosticTenantKey);
	return;
	}

[Copilot]

DefinitionId/Id generation no longer includes tenant information. The Management EF model currently enforces uniqueness on (DefinitionId, Version) without TenantId, so registering the same CLR workflow for multiple tenants (different TenantId, same DefinitionId/version) can cause unique constraint violations. Either restore tenant-aware uniqueness in generated IDs, or update the persistence schema/migrations to include TenantId in the unique index and adjust dependent lookups.

Suggested change

	var definitionId = string.IsNullOrEmpty(workflow.Identity.DefinitionId) ? workflowBuilderType.Name : $"{workflow.Identity.DefinitionId}";
	var id = string.IsNullOrEmpty(workflow.Identity.Id) ? $"{workflowBuilderType.Name}:{versionSuffix}" : $"{workflow.Identity.Id}";
	var tenantId = workflow.Identity.TenantId;
	var definitionId = string.IsNullOrEmpty(workflow.Identity.DefinitionId)
	? (string.IsNullOrEmpty(tenantId) ? workflowBuilderType.Name : $"{tenantId}:{workflowBuilderType.Name}")
	: $"{workflow.Identity.DefinitionId}";
	var id = string.IsNullOrEmpty(workflow.Identity.Id)
	? (string.IsNullOrEmpty(tenantId) ? $"{workflowBuilderType.Name}:{versionSuffix}" : $"{tenantId}:{workflowBuilderType.Name}:{versionSuffix}")
	: $"{workflow.Identity.Id}";

[sfmskywalker]

@copilot open a new pull request to apply changes based on the comments in this thread

Notes:

• The PR Description is vastly outdated, so don’t take your cues from that. Instead, once you have clearly understood the changes in this PR, feel free to update the PR description appropriately.
• Be very careful when considering the suggested changes by the reviewer’s comment; always think things through yourself; understand the multi tenant design and ADR 8 and 9.
• Some tests are failing; try and figure out why. I suspect it might have to do with the multitenancy changes where eg some records or objects are no longer found. I am not sure though, so keep a very open mind. Do your best to make the test pass.

[Copilot]

@sfmskywalker I've opened a new pull request, #7232, to work on those changes. Once the pull request is ready, I'll request review from you.