fix(mongodb): preserve and resolve tenant-agnostic ("*") entities

[arledesma]

Summary

MongoDB multitenancy handling was inconsistent: tenant-agnostic (TenantId == "*") workflow definitions could be overwritten on write and excluded from tenant-scoped reads. This made CLR workflows unreachable under concrete tenants.

This change now:

• preserves every explicit tenant ID, including "*", and stamps only documents whose tenant ID is null
• includes tenant-agnostic documents in tenant-scoped reads
• keeps tenant-scoped deletes strict, including the final delete filter, so a shared custom key cannot remove an agnostic or other-tenant document
• documents direct-collection bypass and legacy null migration behavior
• adds container-backed regression coverage for save, read, custom-key delete, ambient-null delete, and explicit agnostic management semantics

Migration note

Existing records whose intended agnostic marker was already clobbered to null are not changed automatically. Deployments should migrate affected records to TenantId = "*" or repopulate the workflow definitions after deployment. New writes preserve the marker.

Validation

• Elsa.MongoDb.UnitTests on .NET 10: 8 passed
• focused tenant-isolation tests: 4 passed
• Elsa.Persistence.MongoDb Release build: .NET 8, 9, and 10 succeeded
• git diff --check: clean

The build still reports repository-existing dependency/nullability/trimming warnings; this PR introduces no new package dependency.

Fixes #158

[arledesma]

@dotnet-policy-service agree company="ContraForce"

[sfmskywalker]

@greptile review

[Copilot]

Pull request overview

Fixes MongoDB multitenancy behavior so tenant-agnostic ("*") entities are preserved on write and resolvable from within a specific tenant on read, aligning MongoDB persistence behavior with the EFCore provider and the documented tenant-agnostic contract.

Changes:

• Update tenant-scoped query filtering to also include tenant-agnostic (TenantId == "*") documents.
• Prevent ApplyTenantId from overwriting documents already marked as tenant-agnostic ("*"), preserving the marker during inserts/updates.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[greptile-apps]

Greptile Summary

This PR fixes MongoDB multi-tenancy handling to align with the EFCore provider's treatment of tenant-agnostic ("*") entities. The changes are confined to two private methods in MongoDbStore<TDocument>.

• Write path (ApplyTenantId): entities already stamped with "*" are no longer overwritten with the ambient tenant ID, so CLR-registered global workflow definitions keep their agnostic marker.
• Read path (GetQueryableCollection): tenant-scoped queries now include TenantId == "*" via a new includeTenantAgnostic parameter (defaulting to true), making global entities visible under any specific tenant.
• Delete path (DeleteWhereAsync): both terminal delete overloads explicitly pass includeTenantAgnostic: false, ensuring a tenant-scoped delete predicate cannot match and destroy globally-shared entities. The second commit in this PR (362c5d1) directly addresses the concern raised in the prior review about deletes inadvertently removing "*" entities.

Confidence Score: 5/5

Safe to merge — the two private methods are changed consistently, all delete overloads protect global entities, and the fix matches documented EFCore provider behaviour.

The write fix, read fix, and delete guard are all internally consistent. The second commit in this PR directly addresses the delete-path concern raised in the prior review. Every DeleteWhereAsync call chain routes through one of the two patched terminal overloads, so no delete path regressed. Read and count operations correctly include '*' entities, keeping counts in sync with result sets.

No files require special attention.

Important Files Changed

Filename

Overview

src/modules/persistence/Elsa.Persistence.MongoDb/Common/MongoDbStore.cs

Three targeted fixes: ApplyTenantId now skips stamping entities already marked ""; GetQueryableCollection includes TenantId == "" in tenant-scoped reads (via new includeTenantAgnostic param, default true); both DeleteWhereAsync implementations explicitly pass includeTenantAgnostic: false so global entities cannot be destroyed by a tenant-scoped delete. All delete call chains route through the two patched overloads.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[GetQueryableCollection called] --> B{tenantAgnostic == true?}
    B -- Yes --> C[Return unfiltered queryable\nall tenants + all '*' entities]
    B -- No --> D{TDocument is Entity?}
    D -- No --> E[Return unfiltered queryable]
    D -- Yes --> F{includeTenantAgnostic?}
    F -- true\ndefault reads --> G["Filter: TenantId == tenantId OR TenantId == '*'"]
    F -- false\ndeletes --> H["Filter: TenantId == tenantId only"]

    subgraph Callers
        I[FindAsync / FindManyAsync / CountAsync / AnyAsync / ListAsync] --> |includeTenantAgnostic=true| F
        J[DeleteWhereAsync both overloads] --> |includeTenantAgnostic=false| F
    end

    subgraph Write path
        K[ApplyTenantId] --> L{entity.TenantId == '*'?}
        L -- Yes --> M[Skip: preserve agnostic marker]
        L -- No --> N[Stamp with tenantId.EmptyToNull]
    end

Loading

_{Reviews (2): Last reviewed commit: "fix(mongodb): exclude tenant-agnostic ("..." | Re-trigger Greptile}

[arledesma]

Addressed the delete-path concern in 362c5d1.

GetQueryableCollection is shared by the read and delete paths, so the read-side || TenantId == "*" branch had also widened DeleteWhereAsync — a tenant-scoped delete could match and remove a shared "*" entity for every tenant.

Fix: added an includeTenantAgnostic flag (default true). Reads still include "*" rows, but both DeleteWhereAsync terminals now pass includeTenantAgnostic: false, so tenant-scoped deletes stay strict to the current tenant — matching the Dapper store. An explicit tenantAgnostic: true delete is unchanged, and the agnostic context (Tenant.Id == "*") can still manage "*" entities.

[sfmskywalker]

@greptile review

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

[arledesma]

@sfmskywalker Just checking in to see if there are any further changes that are required for either this PR or #159

[greptile-apps]

PR author is not in the allowed authors list.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/modules/persistence/Elsa.Persistence.MongoDb/Common/MongoDbStore.cs:448

• When includeTenantAgnostic is false (delete paths), this filter excludes TenantId == "*" even when there is no current tenant (tenantId is null). Since reads in the same method include "*" when tenantId is null, this can make "*" entities visible but not deletable in tenant-less scenarios. Consider only excluding "*" when a concrete tenant id is present.

            // Deletes pass includeTenantAgnostic: false so a tenant-scoped delete cannot remove a
            // shared "*" entity (the Dapper store keeps tenant-scoped deletes strict).
            queryable = includeTenantAgnostic
                ? queryable.Where(x => (x as Entity)!.TenantId == tenantId || (x as Entity)!.TenantId == Tenant.AgnosticTenantId)
                : queryable.Where(x => (x as Entity)!.TenantId == tenantId);

[arledesma]

@sfmskywalker rebased the branch. no other changes, but it looks like the PR workflow may require another review.

[sfmskywalker]

Reviewed current head d489a75 against #158 and current main. Explicit and agnostic tenant IDs are preserved, only null IDs are stamped, tenant reads include shared rows, and both delete paths retain a final tenant predicate. Container-backed validation passes all 8 MongoDB unit tests; the persistence module builds for .NET 8, 9, and 10. No actionable root-review findings remain.

[Copilot]

🟢 Approved

The implementation matches the stated tenant semantics and includes focused MongoDB regression coverage.

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0 new
• Review effort level: Balanced