fix(permissions): manage admin-authored group_type_role member rights (#65)

docs/permissions.md

@@ -270,10 +270,12 @@ to be accepted by `ct plan` (the round trip is locked by tests):
   - A scoped right granted **globally** in CT (row with no `dataId`) is a
     `WARNING` comment either way — the DSL deliberately cannot declare a
     global grant of a scoped right.
-- **Not-writable rights become `NOTE` comments.** On `group_type_role`, rights
-  with `authId >= 10000` (the `churchdb:+…` family) are readable via inheritance
-  but rejected at plan time (see "Domain rules" below), so they are never
-  emitted as grants.
+- **System-baseline and inherited rows are dropped, not emitted.** Adoption runs
+  the live rows through `normalizeActual`, which excludes the self-re-adding
+  system baseline (`modifiedPid === -1`) and any `isInherited` row. Admin-authored
+  direct grants — **including** the writable `authId >= 10000` `churchdb:+…`
+  member rights CT lets you set on `group_type_role` — are emitted as active
+  grants (no authId cutoff; #65).
 - **Only `group_role` / `group_type_role`** are valid; people domains are
   refused (the same hard boundary as everywhere else).
 
@@ -293,11 +295,14 @@ and `ct plan`.
 
 ## Domain rules (validated, throw on violation)
 
-- **`group_type_role` requires `authId < 10000`.** Rights with `authId >=
-  10000` are not writable through this domain type in ChurchTools; declaring
-  one under `ct.groupTypeRole` throws at plan/evaluation time
-  (`src/permissions/plan.ts`, `desiredTuples`). Use `ct.groupRole` for those
-  rights instead.
+- **No authId cutoff.** Admin-authored member rights (`authId >= 10000`, the
+  `churchdb:+…` family) ARE writable on `group_type_role` and can be declared
+  under `ct.groupTypeRole` — verified live (eqrm prod `group_type_role/9` carries
+  24 such admin-set rows). What ct never reconciles is decided by the live row's
+  flags, not its authId: `normalizeActual` drops the system baseline
+  (`modifiedPid === -1`) and every `isInherited` row, so those are neither adopted
+  nor revoked (#65). Earlier versions blocked `authId >= 10000` outright — that was
+  too broad and is removed.
 - **Revocation is a later extension, not exposed yet.** `GrantTuple.type` is
   typed as `"grant" | "revoke"`, but the DSL and `desiredTuples` currently
   only ever *emit* `"grant"` tuples — there is no config-level way to declare

src/permissions/adopt.ts

@@ -17,7 +17,7 @@
 import type { State } from "../state/state.js";
 import { findByTypeId } from "../state/state.js";
 import { CATALOG } from "./catalog.js";
-import { normalizeActual, isInheritedOnlyRight, type DomainType, type GrantTuple, type RawPermission } from "./grants.js";
+import { normalizeActual, type DomainType, type GrantTuple, type RawPermission } from "./grants.js";
 
 /** DSL function name for each domain type — the call the emitted block should be pasted as. */
 const DSL_FN: Record<DomainType, string> = {
@@ -113,7 +113,7 @@ export function emitAdoptedGrants(args: {
   } else {
     body.push("  grants: [");
     for (const g of collapse(grants)) {
-      const r = grantLines(g, rev, state, domainType);
+      const r = grantLines(g, rev, state);
       body.push(...r.lines);
       if (r.omitted) omitted += 1;
     }
@@ -159,14 +159,13 @@ interface GrantLinesResult {
  *
  * Round-trip invariant (locked by tests): every ACTIVE (non-comment) line this emits must pass
  * `desiredTuples` (`src/permissions/plan.ts`) without throwing — anything the planner would
- * reject (group_type_role authIds >= 10000, a scoped right without a declarable scope, an
- * unscoped right carrying dataIds) is emitted as a comment instead.
+ * reject (a scoped right without a declarable scope, an unscoped right carrying dataIds) is emitted
+ * as a comment instead.
  */
 function grantLines(
   g: CollapsedGrant,
   rev: Map<number, ReverseEntry>,
   state: State,
-  domainType: DomainType,
 ): GrantLinesResult {
   const entry = rev.get(g.authId);
   if (!entry) {
@@ -181,21 +180,10 @@ function grantLines(
     };
   }
 
-  // Mirror the planner's writability guard: `desiredTuples` throws for group_type_role grants
-  // with authId >= 10000 (the `churchdb:+…` family) — they are readable here via inheritance but
-  // not writable on this domain type, so emitting them would break the paste-and-plan round trip.
-  // The reconciler excludes these SAME rows from the actual set via the shared predicate (#65).
-  if (isInheritedOnlyRight(domainType, g.authId)) {
-    return {
-      omitted: true,
-      lines: [
-        `    // NOTE: "${entry.name}" (authId ${g.authId}) is not writable on group_type_role — this right`,
-        "    //       reaches roles via inheritance only (authId >= 10000). Declaring it here would be",
-        "    //       rejected at plan time, so it is intentionally left as a comment.",
-      ],
-    };
-  }
-
+  // No authId cutoff (#65): the grants reaching here already passed `normalizeActual` (line ~100),
+  // which drops the system baseline (`modifiedPid === -1`) and every `isInherited` row — so what is
+  // left is admin-authored DIRECT grants, INCLUDING the writable `authId >= 10000` group-member rights
+  // Equippers curates on group_type_role. Those ARE declarable and are emitted as active grants.
   if (entry.scopeField != null && entry.scopeField !== GROUP_SCOPE_FIELD) {
     // Scoped right whose scope dimension is NOT a group (e.g. "cc_securitylevel", "cdb_comment_viewer")
     // — its dataIds name something this tool has no managed/logical representation for (a security

src/permissions/grants.ts

@@ -5,31 +5,17 @@
  */
 export type DomainType = "group_role" | "group_type_role";
 
-/**
- * Smallest authId of the inheritance-only rights family (the `churchdb:+…` rights). These reach
- * roles via inheritance and are NOT writable on a group_type_role domain — CT exposes them as
- * readable rows there, but any write is rejected.
- */
-export const INHERITED_RIGHT_MIN_AUTH_ID = 10000;
-
-/**
- * Whether a right is readable-but-not-writable on this domain type: it reaches roles via
- * inheritance only, so it can neither be declared (`desiredTuples` throws for it) nor adopted
- * (adopt emits it as a NOTE comment, never an active grant).
- *
- * SINGLE SOURCE OF TRUTH — this predicate MUST be shared by three sites that would otherwise drift
- * and reopen issue #65:
- *  1. `desiredTuples` (plan.ts) rejects declaring such a right,
- *  2. `emitAdoptedGrants` (adopt.ts) excludes it from the emitted block, and
- *  3. `buildPermissionPlan` (plan.ts) excludes matching LIVE rows from the ACTUAL diff set.
- * If (1)/(2) exclude a right but (3) keeps it, the plan demands a revoke the DSL can never satisfy
- * — pasted adopt output can never converge to a no-op (the #65 bug).
- *
- * Today only group_type_role carries such rights (authId >= 10000).
- */
-export function isInheritedOnlyRight(domainType: DomainType, authId: number): boolean {
-  return domainType === "group_type_role" && authId >= INHERITED_RIGHT_MIN_AUTH_ID;
-}
+// What reconciliation owns vs. leaves untouched is decided SOLELY by `normalizeActual` below:
+// it drops the self-re-adding system baseline (`modifiedPid === -1`) and any `isInherited` row, so
+// only admin-authored DIRECT grants are managed. There is intentionally NO authId-based cutoff.
+//
+// (History, #65: an earlier `isInheritedOnlyRight(domainType, authId) = authId >= 10000` predicate
+// gated declaration/adoption/diff on the authId. That was too broad — it also excluded the
+// admin-authored `authId >= 10000` group-member rights (e.g. "Add group members" 10107) that CT DOES
+// let you write on a group_type_role domain and that Equippers prod actually curates. Verified live
+// on eqrm prod `group_type_role/9`: 24 such rows carry `isInherited:false` and `modifiedPid` 1/3891.
+// The real, verified boundary is inheritance + system-baseline, which normalizeActual already
+// enforces on every path that reads actuals — the #65 no-op guarantee holds without the authId cutoff.)
 
 export interface GrantTuple {
   authId: number;

src/permissions/plan.ts

@@ -11,7 +11,7 @@ import type { DesiredResource } from "../engine/types.js";
 import { resolveAuthId, CATALOG_META, KNOWN_AUTH_IDS } from "./catalog.js";
 import { compareVersions } from "../api/version.js";
 import { resolveScope } from "./scope.js";
-import { normalizeActual, diffGrants, isInheritedOnlyRight, INHERITED_RIGHT_MIN_AUTH_ID, type GrantTuple, type GrantDiff, type DomainType, type RawPermission } from "./grants.js";
+import { normalizeActual, diffGrants, type GrantTuple, type GrantDiff, type DomainType, type RawPermission } from "./grants.js";
 import type { DesiredPermission } from "./types.js";
 import { Resolver } from "../resolve/resolver.js";
 import { isPendingRef, refKey, refLabel, type Ref } from "../resolve/refs.js";
@@ -43,9 +43,6 @@ export function desiredTuples(
   return p.grants.flatMap((g): GrantTuple[] => {
     const name = typeof g === "string" ? g : g.right;
     const entry = resolveAuthId(name);
-    if (isInheritedOnlyRight(p.domainType, entry.authId)) {
-      throw new Error(`${p.domainType} "${p.key}": "${name}" (authId ${entry.authId}) is not writable — ${p.domainType} requires authId < 10000.`);
-    }
     if (typeof g === "string") {
       // A scoped right declared as a bare string would emit `dataId: []` — a silent GLOBAL grant.
       // Refuse it: a scoped right must be declared as `{ right, scope: [...] }` so the scope is explicit.
@@ -188,37 +185,25 @@ export async function buildPermissionPlan(
     const all = byType.get(p.domainType);
     if (all == null) continue; // fetch failed for this domainType — recorded above
     const normalizedAll = normalizeActual(all.filter((r) => r.domainId === p.domainId));
-    // Inherited-rights guard (#65): a group_type_role reads back rights it acquires via inheritance
-    // (the `churchdb:+…` family, authId >= 10000) as live rows, but those rights are NOT writable on
-    // this domain — `desiredTuples` refuses to declare them and `ct adopt grants` emits them only as
-    // NOTE comments. If they stayed in the ACTUAL set they would have no desired counterpart and land
-    // in `toDelete`, so `ct plan` would demand revokes the DSL can never satisfy — a no-op would be
-    // impossible for any role domain with inheritance. Exclude them here (below, after the
-    // unknown-authId guard), mirroring the exact predicate the declaration/adopt sides use, and
-    // surface a single informational summary line.
+    // Actuals are already filtered by `normalizeActual`: the self-re-adding system baseline
+    // (`modifiedPid === -1`) and every `isInherited` row are dropped, so what remains is admin-authored
+    // DIRECT grants — INCLUDING the writable `authId >= 10000` group-member rights Equippers curates on
+    // group_type_role (e.g. "Add group members" 10107). Those are reconciled like any other grant; there
+    // is no authId cutoff (see grants.ts — the old `authId >= 10000` exclusion was too broad, #65).
     //
     // Unknown-authId guard (#25): a live GRANT whose authId is absent from the catalog cannot be
     // named or described. Keep it OUT of the diff — otherwise, having no desired counterpart, it
     // would land in `toDelete` and `ct apply` would silently revoke a right we cannot even name.
     // Instead, warn (naming authId + domain) and leave it untouched. Idempotent: excluded every run.
     // (Revoke/deny rows with an unknown authId are already `preserved` by diffGrants, so ignore them
     // here — only unknown grant rows are the churn/silent-revoke hazard.)
-    //
-    // Order matters: the unknown-authId check runs FIRST so an unknown authId that merely happens to
-    // be >= 10000 is reported as unknown, not misclassified as an inherited catalog right. Only KNOWN
-    // rights reach the inherited-rights check.
     const knownActual: GrantTuple[] = [];
     const unknownAuthIds = new Set<number>();
-    const inheritedAuthIds = new Set<number>();
     for (const t of normalizedAll) {
       if (t.type === "grant" && !KNOWN_AUTH_IDS.has(t.authId)) {
         unknownAuthIds.add(t.authId);
         continue;
       }
-      if (isInheritedOnlyRight(p.domainType, t.authId)) {
-        inheritedAuthIds.add(t.authId);
-        continue;
-      }
       knownActual.push(t);
     }
     for (const authId of [...unknownAuthIds].sort((a, b) => a - b)) {
@@ -228,13 +213,6 @@ export async function buildPermissionPlan(
           `(\`npm run regenerate:permission-catalog\`) if this right should be manageable.`,
       );
     }
-    if (inheritedAuthIds.size > 0) {
-      warnings.push(
-        `${p.domainType} #${p.domainId} ("${p.key}"): ${inheritedAuthIds.size} inherited right(s) ` +
-          `(authId >= ${INHERITED_RIGHT_MIN_AUTH_ID}) are readable but not writable on ${p.domainType} — left untouched ` +
-          `(never revoked); they reach roles via inheritance only.`,
-      );
-    }
     items.push({ key: p.key, domainType: p.domainType, domainId: p.domainId, diff: diffGrants(desiredTuples(p, state, declaredGroupKeys), knownActual) });
   }
   return { items, fetchErrors, warnings };