# Group Membership Closure ## Status Issue: [#586](https://github.com/pingcap-inc/NexusCRM/issues/586) Decision: use a denormalized L0 substrate table, `_System_GroupMembershipClosure`, instead of recursive CTEs in RLS and sharing read paths. ## Problem `GroupMember` is polymorphic: `UserOrGroupId` may point at either a `User` or a `Group`. Current read paths treat it as a direct user-to-group edge. If user `U` belongs to group `A`, and group `A` belongs to group `B`, reads that check group `B` do not grant `U` access. The broken surfaces are: - SQL RLS fragments in `backend/internal/domain/rls/rls.go` - single-record permission checks and pre-resolved SOQL helpers in `backend/internal/infrastructure/persistence/perm_roles.go` - sharing-rule target/source checks in `backend/internal/features/permissions` - folder access through `GetUserGroupIDs` - workflow alert group recipient resolution ## Data Model Add `_System_GroupMembershipClosure` as a system-core auth table: - `Id`: deterministic row id derived from `(GroupId, MemberType, MemberId)` - `GroupId`: containing group - `MemberId`: transitive member id - `MemberType`: `User` or `Group` - `Depth`: shortest active path length from `GroupId` to `MemberId` - `CreatedDate`, `LastModifiedDate`: rebuild timestamp Indexes: - unique `(GroupId, MemberType, MemberId)` - lookup `(MemberType, MemberId, GroupId)` for "groups containing user" - lookup `(GroupId, MemberType)` for "members of group" Rows are derived state. Runtime/admin APIs must not author them directly. Their authoritative inputs are `GroupMember`, `Group`, and `User`. With the closure read path in place, public `GroupMember.UserOrGroupId` CRUD may author either `User` or `Group` targets through the same polymorphic validation and server-derived discriminator contract as import/repair paths. ## Maintenance Use one authoritative rebuild helper in the persistence layer: 1. Read active `Group`, active `User`, and active `GroupMember` rows. 2. Classify each direct `GroupMember.UserOrGroupId` as `Group` first, then `User`; ignore dangling members. 3. Build an in-memory adjacency list of active group-to-member edges. 4. For each active group, breadth-first traverse nested groups: - emit direct users/groups at `Depth=1` - emit deeper users/groups with the shortest depth - do not emit self-membership rows - track traversed groups so cycles terminate deterministically 5. Runtime mutations on `GroupMember`, `Group`, or `User` mark the closure dirty inside a source-aware transaction. The transaction acquires the advisory lock `nexuscrm_group_membership_closure` before opening the SQL transaction, rebuilds once before the top-level commit when dirty, and releases the lock only after commit/rollback returns. Repair/rebuild jobs run the same helper for backfill: they acquire the same lock, rebuild inside a SQL transaction, and release the lock after commit/rollback. This is write-amplifying, but read paths stay simple indexed lookups. Issue #586 prioritizes RLS and sharing read latency; current group cardinality is expected to be small enough that full rebuilds on source mutations are acceptable. A future optimization can make the helper incremental without changing the read contract. ## Write Hooks Mark the closure dirty after successful physical writes to source tables: - `GroupMember`: insert, update, delete/soft-delete, bulk insert - `Group`: insert, delete/soft-delete, bulk insert, and updates that change `IsDeleted` - `User`: insert, delete/soft-delete, bulk insert, and updates that change `IsDeleted` `User.IsActive` does not affect membership closure. For this substrate, "active" means `IsDeleted = 0 OR IsDeleted IS NULL`. Single-record write paths open a source-aware transaction for those source objects. Composite/all-or-none paths pre-scan their object names and use the same source-aware wrapper when any source object is present, so nested writes share the top-level lock state. A source-table mutation that runs inside an active SQL transaction without closure lock state fails closed instead of silently bypassing maintenance. Internal `PhysicalDeleteRaw` rejects source tables for the same reason. `setup repair`, `setup apply`, `init-l0`, and explicit derived-state rebuilds also rebuild the closure to repair warm-upgraded or drifted databases. Those control-plane rebuilds must use the same lock and transactional rebuild wrapper as runtime source writes. Because the runtime implementation uses full replacement rather than incremental deltas, source-aware transactions acquire a named MySQL/TiDB advisory lock before the transaction starts and release it after commit/rollback. That prevents concurrent mutations from rebuilding from different snapshots and losing each other's derived rows. ## Read Contract All existing single-hop reads move to closure reads: - "groups containing user U": `GroupId FROM _System_GroupMembershipClosure WHERE MemberType='User' AND MemberId=U` - "members of group G": `MemberId FROM _System_GroupMembershipClosure WHERE GroupId=G` - email group recipients: `MemberId WHERE GroupId=G AND MemberType='User'` - manual shares: `ShareWithGroupId IN (groups containing user U)` - group-owned record visibility: `OwnerId IN (groups containing user U)` - sharing-rule source group owner expansion: `OwnerId IN (members of group G)`, without a member-type filter, preserving existing behavior for user-owned and group-owned records. ## Cycle And Delete Semantics For a cycle `A -> B -> A`, traversal marks groups as visited and skips self-membership output. Users reachable through the cycle are still emitted once with their shortest depth. Soft-deleted `GroupMember` rows are excluded. Soft-deleted `User` and `Group` rows are excluded from classification and traversal. A rebuild after soft-delete removes stale transitive rows. ## Validation Plan - Unit test closure construction for depth 1, depth 2, depth 3, cycle handling, soft-deleted direct edges, and dangling members. - Unit test source-impact classification for `GroupMember`, `Group`, `User`, non-source objects, and `IsDeleted`-only update behavior on `Group`/`User`. - Unit test source-aware transaction behavior: lock acquisition, rebuild once before commit, nested source mutation fail-closed without state, and nested mutation sharing state when wrapped. - Unit test standalone repair rebuilds acquire the same advisory lock, rebuild inside a SQL transaction, roll back on rebuild errors, and release the lock. - Unit test the persistence rebuild loaders exclude soft-deleted `User`, `Group`, and `GroupMember` rows from actual SQL inputs. - Unit test RLS SQL fragments use `_System_GroupMembershipClosure`. - Unit test permission repository queries consume the closure table for membership, manual share levels, user group ids, and manual share record ids. - Run: - `(cd backend && go test ./...)` - `(cd shared && go test ./...)` - `(cd frontend && npm run build)` - `git diff --check` ## Design Confidence Pass Pass 1 finding: using recursive CTEs in every read path would duplicate graph logic and force top-level `WITH` plumbing through builders that currently emit subqueries. The issue comments already locked this out. Pass 2 finding: maintaining closure only during `setup repair` would make fresh runtime `GroupMember` writes invisible until the next repair. Runtime write hooks are required. Pass 3 finding: filtering every group-member read to `MemberType='User'` would regress existing owner-source behavior where `OwnerId` can be a group. The read contract filters by member type only when the caller truly needs users. Pass 4 finding: emitting self rows during cycles would make a group appear to be its own member. Skipping self-membership rows while still traversing non-origin groups preserves reachability without creating self-grants. Pass 5 finding: `GroupMember.UserOrGroupId.referenceTo` must name both stored targets so generic repair and public CRUD can resolve nested-group rows through one polymorphic lookup contract. During the #586 transition, a writable-reference-target subset kept public authoring narrow; issue #587 removes that subset once closure reads make Group-target writes safe. Pass 6 finding: a full-table rebuild inside each write is only correct under concurrent runtime writes if those writes are serialized. The implementation therefore locks the whole `GroupMember` mutation transaction, not only the rebuild statement. After these passes, the design has a single source of truth, one read contract, finite cycle behavior, and a repair path for warm upgrades.