From db9d5ea2795a35b69f30e87e9d43d3488856abf4 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 9 Jul 2026 08:56:40 +0200 Subject: [PATCH 1/5] feat(refs): portable logical references via a shared per-host resolver (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add Ref sentinels + ref.* helper (src/resolve/refs.ts) and a per-host Resolver (src/resolve/resolver.ts) sourcing ids from managed desired∪state (pending for same-run targets) then live catalogs (campus/group-type/group-status/role-def), throwing on unknown/ambiguous. DSL sugars campus/groupType/status into Ref-valued id fields and accepts groupType (group_type_role) + gated group+role (group_role). Wire the resolver into buildPlan (resolution pass before computePlan), permission domainId resolution, apply-time pending re-resolution, and pending plan rendering. --- src/commands/apply.ts | 7 +- src/commands/plan.ts | 8 +- src/config/context.ts | 111 +++++++++++++++--- src/config/query.ts | 5 + src/engine/build.ts | 22 +++- src/engine/execute.ts | 13 ++- src/engine/render.ts | 7 +- src/permissions/plan.ts | 57 +++++++++- src/permissions/types.ts | 8 +- src/resolve/refs.ts | Bin 0 -> 6115 bytes src/resolve/resolver.ts | 236 +++++++++++++++++++++++++++++++++++++++ tests/context.test.ts | 26 ++++- 12 files changed, 468 insertions(+), 32 deletions(-) create mode 100644 src/resolve/refs.ts create mode 100644 src/resolve/resolver.ts diff --git a/src/commands/apply.ts b/src/commands/apply.ts index 7443f30..b11f220 100644 --- a/src/commands/apply.ts +++ b/src/commands/apply.ts @@ -5,6 +5,7 @@ import { resolveConfig } from "../config.js"; import { loadState, resolveStatePath, saveState } from "../state/state.js"; import { loadConfig, resolveConfigPath } from "../config/load.js"; import { buildPlan } from "../engine/build.js"; +import { Resolver } from "../resolve/resolver.js"; import { executePlan } from "../engine/execute.js"; import { runPostApplyHooks } from "../engine/synthetic.js"; import { writeBackup } from "../engine/backup.js"; @@ -53,12 +54,14 @@ export function applyCommand(): Command { const state = await loadState(statePath, config.host); const { client } = await authedSession(); + // One shared resolver (#20) across both concurrent plans — see commands/plan.ts. + const resolver = new Resolver({ client, state, desired, host: config.host }); // Independent fetches: the resource plan and the permission plan (whose instance-wide // /permissions/ reads are slow) run concurrently rather than back-to-back. const [{ plan, actual, fetchErrors }, { items: permItems, fetchErrors: permFetchErrors }] = await Promise.all([ - buildPlan(client, state, desired, { configDir }), - buildPermissionPlan(client, state, permissions, desired), + buildPlan(client, state, desired, { configDir, resolver }), + buildPermissionPlan(client, state, permissions, desired, resolver), ]); const allFetchErrors = [...fetchErrors, ...permFetchErrors]; diff --git a/src/commands/plan.ts b/src/commands/plan.ts index cf51780..29e0d69 100644 --- a/src/commands/plan.ts +++ b/src/commands/plan.ts @@ -4,6 +4,7 @@ import { resolveConfig } from "../config.js"; import { loadState, resolveStatePath } from "../state/state.js"; import { loadConfig, resolveConfigPath } from "../config/load.js"; import { buildPlan } from "../engine/build.js"; +import { Resolver } from "../resolve/resolver.js"; import { renderPlan } from "../engine/render.js"; import { buildPermissionPlan } from "../permissions/plan.js"; import { renderPermissionPlan } from "../permissions/render.js"; @@ -29,11 +30,14 @@ export function planCommand(): Command { const state = await loadState(resolveStatePath(opts.state), config.host); const { client } = await authedSession(); + // One shared resolver (#20): buildPlan and buildPermissionPlan run concurrently, so a single + // instance means each master-data catalog is fetched at most once (cache is Promise-keyed). + const resolver = new Resolver({ client, state, desired, host: config.host }); // Independent fetches run concurrently (see commands/apply.ts). const [{ plan, fetchErrors }, { items: permItems, fetchErrors: permFetchErrors }] = await Promise.all([ - buildPlan(client, state, desired, { configDir }), - buildPermissionPlan(client, state, permissions, desired), + buildPlan(client, state, desired, { configDir, resolver }), + buildPermissionPlan(client, state, permissions, desired, resolver), ]); if (opts.json) { out({ plan, permissions: permItems }); diff --git a/src/config/context.ts b/src/config/context.ts index 8f97754..8e2be31 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -14,10 +14,14 @@ import type { DesiredResource, DynamicSpec, DynamicStatus } from "../engine/types.js"; import type { DomainType } from "../permissions/grants.js"; import type { DesiredPermission, Grant } from "../permissions/types.js"; +import { isRef, ref, refKey, type Ref } from "../resolve/refs.js"; // Re-exported so a config file can pull the query DSL from the same module as // `ConfigContext`: `import { q, churchQuery } from "../../src/config/context.js"`. export { q, churchQuery } from "./query.js"; export type { QueryNode } from "./query.js"; +// Re-exported so a config can pull the logical-reference helper from the same module: it turns a +// name/key into an inert `Ref` sentinel the per-host resolver later maps to a numeric id (#20). +export { ref } from "../resolve/refs.js"; const DYNAMIC_STATUSES = ["active", "inactive", "manual", "none"] as const; @@ -43,10 +47,66 @@ export interface ResourceInput { export interface PermissionInput { key: string; - id: number; + /** Numeric domainId (the escape hatch). Mutually exclusive with the logical forms below. */ + id?: number; + /** `group_type_role`: the group type by name/key — sugars into a Ref-valued domainId (#20). */ + groupType?: string; + /** `group_role`: the group by key (paired with `role`) — GATED, see `ref.groupRole`/#25. */ + group?: string; + /** `group_role`: the role name (paired with `group`) — GATED, see `ref.groupRole`/#25. */ + role?: string; grants: Grant[]; } +/** Logical id-field sugar for declarations: a named string field → a Ref-valued numeric id field. */ +const ID_SUGAR: Record Ref }> = { + campus: { idField: "campusId", make: ref.campus }, + groupType: { idField: "groupTypeId", make: ref.groupType }, + status: { idField: "groupStatusId", make: ref.status }, +}; + +/** The numeric id fields a declaration may carry — each accepts a number, `null`, or a {@link Ref}. */ +const ID_FIELDS = ["campusId", "groupTypeId", "groupStatusId"] as const; + +/** Canonical string for a domainId (number or Ref) — keys the eval-time duplicate-target guard. */ +function domainKeyPart(domainId: number | Ref): string { + return typeof domainId === "number" ? String(domainId) : refKey(domainId); +} + +/** + * Resolve a permission declaration's domain to a numeric id (escape hatch) or a {@link Ref} (#20): + * - `group_type_role`: numeric `id`, or logical `groupType: ""` → `ref.groupType(...)`. + * - `group_role`: numeric `id`, or logical `group` + `role` → `ref.groupRole(...)` (GATED — the + * resolver rejects it at plan time with a "pass a numeric id" error; see #25). + * Declaring both a numeric `id` and a logical form is a conflict. + */ +function resolveDomainInput(domainType: DomainType, input: PermissionInput): number | Ref { + const hasId = input.id !== undefined; + const bothError = (logical: string): Error => + new Error(`${domainType} "${input.key}": declare either "id" (numeric) or ${logical} (logical), not both.`); + if (domainType === "group_type_role") { + if (input.groupType !== undefined) { + if (hasId) throw bothError('"groupType"'); + if (typeof input.groupType !== "string" || !input.groupType) + throw new Error(`${domainType} "${input.key}": "groupType" must be a non-empty group-type key.`); + return ref.groupType(input.groupType); + } + } else { + // group_role + if (input.group !== undefined || input.role !== undefined) { + if (hasId) throw bothError('"group" + "role"'); + if (typeof input.group !== "string" || !input.group || typeof input.role !== "string" || !input.role) + throw new Error(`${domainType} "${input.key}": "group" and "role" must both be non-empty strings.`); + return ref.groupRole(input.group, input.role); + } + } + if (typeof input.id !== "number" || !Number.isFinite(input.id)) { + const logical = domainType === "group_type_role" ? '"groupType"' : '"group" + "role"'; + throw new Error(`${domainType} "${input.key}": provide a numeric "id" (the domainId) or the logical ${logical} form.`); + } + return input.id; +} + export interface ConfigContext { campus(input: ResourceInput): void; group(input: ResourceInput): void; @@ -75,19 +135,35 @@ function toDesired(type: string, input: ResourceInput): DesiredResource { if (parents !== undefined && (!Array.isArray(parents) || parents.some((p) => typeof p !== "string"))) { throw new Error(`${type} "${key}": "parents" must be an array of string group keys.`); } - // Campus assignment is a numeric escape hatch only (mirrors `groupTypeId`): `campusId: `. - // A logical `campus: "mainz"` reference is #20's resolver, not built yet — reject it up front - // rather than let an un-diffable `campus` field slip into the bag and drift against the - // managed-only actual forever. Also pin `campusId`'s type so a stray string fails at eval time. - if (fields.campus !== undefined) { + // Logical id-field sugar (#20): a named string field (`campus`/`groupType`/`status`) sugars into + // a Ref-valued numeric id field (`campusId`/`groupTypeId`/`groupStatusId`). The per-host resolver + // turns the Ref into a real id at plan time. Declaring BOTH forms (`campus` + `campusId`) is a + // conflict — reject it rather than silently pick one. Numeric ids still pass straight through. + for (const [logical, { idField, make }] of Object.entries(ID_SUGAR)) { + if (fields[logical] === undefined) continue; + if (fields[idField] !== undefined) { + throw new Error( + `${type} "${key}": declare either "${logical}" (logical reference) or "${idField}" (numeric id), not both.`, + ); + } + const value = fields[logical]; + if (typeof value !== "string" || value.length === 0) { + throw new Error(`${type} "${key}": "${logical}" must be a non-empty string key (e.g. "${logical}: \\"mainz\\"").`); + } + fields[idField] = make(value); + delete fields[logical]; + } + // After sugar, each id field must be a number (escape hatch), null (clear), or a Ref (logical). + // A stray string (e.g. `campusId: "4"`) is rejected so a mistyped id fails at eval, not silently. + for (const idField of ID_FIELDS) { + const value = fields[idField]; + if (value === undefined || value === null || typeof value === "number" || isRef(value)) continue; throw new Error( - `${type} "${key}": logical campus references ("campus") are not supported yet — ` + - `use a numeric "campusId" (the existing CT campus id). Logical references land with #20.`, + `${type} "${key}": "${idField}" must be a number (the CT id), null to clear, or a logical ` + + `reference (use the "${Object.entries(ID_SUGAR).find(([, s]) => s.idField === idField)?.[0] ?? "logical"}" ` + + `field, or ref.*).`, ); } - if (fields.campusId !== undefined && fields.campusId !== null && typeof fields.campusId !== "number") { - throw new Error(`${type} "${key}": "campusId" must be a number (the CT campus id) or null to clear.`); - } // `dynamic` is a synthetic field for auto-groups, handled separately from the plain diffed // field bag. Opt-in: `undefined` means "not a dynamic group" (mirrors `parents`). let dynamicSpec: DynamicSpec | undefined; @@ -176,8 +252,7 @@ export function createContext(): { (input: PermissionInput): void => { if (typeof input.key !== "string" || !input.key) throw new Error(`${domainType} declaration missing a string "key".`); - if (typeof input.id !== "number" || !Number.isFinite(input.id)) - throw new Error(`${domainType} "${input.key}": "id" must be a number (the domainId).`); + const domainId = resolveDomainInput(domainType, input); if (!Array.isArray(input.grants)) throw new Error(`${domainType} "${input.key}": "grants" must be an array.`); for (const g of input.grants) { const right = typeof g === "string" ? g : g?.right; @@ -188,15 +263,19 @@ export function createContext(): { } if (seen.has(input.key)) throw new Error(`Duplicate logical key "${input.key}" in config.`); seen.add(input.key); - const domainKey = `${domainType}:${input.id}`; + // Duplicate-target guard, keyed by the canonical domain string (numeric id or Ref key). This + // catches obvious eval-time collisions early; the authoritative check runs post-resolution in + // buildPermissionPlan (two different refs, or a ref and a number, can resolve to the same id). + const domainKey = `${domainType}:${domainKeyPart(domainId)}`; const existingKey = seenDomains.get(domainKey); if (existingKey) { + const label = typeof domainId === "number" ? `#${domainId}` : refKey(domainId); throw new Error( - `Duplicate permission target: ${domainType} #${input.id} is declared by both "${existingKey}" and "${input.key}". Merge their grants into one declaration.`, + `Duplicate permission target: ${domainType} ${label} is declared by both "${existingKey}" and "${input.key}". Merge their grants into one declaration.`, ); } seenDomains.set(domainKey, input.key); - permissions.push({ key: input.key, domainType, domainId: input.id, grants: input.grants }); + permissions.push({ key: input.key, domainType, domainId, grants: input.grants }); }; // Every type emitted here MUST have an apply tier in engine/graph.ts TYPE_TIER // (locked by tests/context.test.ts), else computePlan rejects it at plan time. diff --git a/src/config/query.ts b/src/config/query.ts index 505809a..e133065 100644 --- a/src/config/query.ts +++ b/src/config/query.ts @@ -11,6 +11,11 @@ * are the observed common case (grouping/returning by `person.id`); override * via `opts` for a query keyed on a different primary entity. */ +// Re-exported so a ruleset built with this DSL can drop a logical reference straight into a `var` +// value: `q.eq("ctgroup.campusId", ref.campus("mainz"))`. The Ref is an inert sentinel the per-host +// resolver turns into a numeric id at plan time (#20) — the numeric escape hatch still works too. +export { ref } from "../resolve/refs.js"; + export interface QueryNode { [op: string]: unknown; } diff --git a/src/engine/build.ts b/src/engine/build.ts index 56e6200..f28a1f4 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -12,6 +12,7 @@ import type { DesiredResource, Plan } from "./types.js"; import { RESOURCES } from "../resources/registry.js"; import { computePlan } from "./plan.js"; import { foldSynthetic } from "./synthetic.js"; +import { Resolver } from "../resolve/resolver.js"; import { mapConcurrent } from "../util/concurrency.js"; import { warn } from "../ui.js"; @@ -85,6 +86,13 @@ export async function fetchActual( export interface BuildOptions { /** Directory of the config file — `{ ref }` ruleset paths resolve relative to it (not the cwd). */ configDir?: string; + /** + * Shared per-host reference resolver (#20). The command layer constructs ONE instance and passes + * it to both `buildPlan` and `buildPermissionPlan` (they run concurrently) so each master-data + * catalog is fetched at most once per run. Omitted → a private resolver is built from this call's + * client/state/desired (fine for tests and single-surface use). + */ + resolver?: Resolver; } export async function buildPlan( @@ -102,6 +110,18 @@ export async function buildPlan( // Synthetic sub-resource fields (parents, dynamic, …) fold into the diff on both sides. const folded = await foldSynthetic({ client, state, desired, actual, configDir: opts.configDir }); fetchErrors.push(...folded.errors); - const plan = computePlan(folded.desired, state, actual, { unresolved, fetchFailed }); + + // Resolution pass (#20): rewrite Ref-valued fields (and the dynamic ruleset, walked deeply) to + // numbers / pending markers AFTER folding, BEFORE computePlan — so the diff stays number↔number. + // Unknown/ambiguous refs THROW here (a config error, not a degrade-and-continue fetch error). + const resolver = opts.resolver ?? new Resolver({ client, state, desired }); + const resolved = await Promise.all( + folded.desired.map(async (d) => { + const fields = await resolver.resolveValue(d.fields, `${d.type} "${d.key}"`); + return fields === d.fields ? d : { ...d, fields: fields as Record }; + }), + ); + + const plan = computePlan(resolved, state, actual, { unresolved, fetchFailed }); return { plan, actual, fetchErrors }; } diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 91c8b46..59e5961 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -15,6 +15,8 @@ import type { FieldChange, Plan } from "./types.js"; import { RESOURCES } from "../resources/registry.js"; import { assertNotPeople } from "./guard.js"; import { isSyntheticField, syntheticField } from "./synthetic.js"; +import { reresolvePendingValue } from "../resolve/resolver.js"; +import { hasPendingRef } from "../resolve/refs.js"; export interface ExecuteDeps { client: Pick; @@ -56,6 +58,11 @@ async function applySyntheticFields( export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise { const { client, state, statePath } = deps; + // Re-resolve any pending logical reference (#20) against the current state before a body/snapshot + // is used. Tier ordering guarantees a referenced target (e.g. a same-run campus, tier 0) is already + // in state by the time its referencer (a group, tier 1) applies. No-op when nothing is pending. + const reresolve = (fields: Record): Record => + hasPendingRef(fields) ? (reresolvePendingValue(fields, state) as Record) : fields; const now = deps.now ?? (() => new Date().toISOString()); const save = deps.save ?? saveState; const created: string[] = []; @@ -91,7 +98,7 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise("POST", spec.collectionPath, body); if (typeof res.id !== "number") { @@ -117,12 +124,12 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise !isSyntheticField(c.field)); if (hasFieldChange) { // PATCH resources take only the changed fields (unchanged/drifted siblings are left alone); // PUT resources replace the whole object, so send actual ∪ changes to preserve those siblings. - const body = spec.updateMethod === "PATCH" ? snapshotFromChanges({}, item.changes) : snapshot; + const body = spec.updateMethod === "PATCH" ? reresolve(snapshotFromChanges({}, item.changes)) : snapshot; const path = spec.itemPath(id); assertNotPeople(path); await client.request(spec.updateMethod, path, body); diff --git a/src/engine/render.ts b/src/engine/render.ts index 9d5cfcf..638ca54 100644 --- a/src/engine/render.ts +++ b/src/engine/render.ts @@ -4,6 +4,7 @@ */ import pc from "picocolors"; import { type Plan, type PlanAction, summarize } from "./types.js"; +import { isPendingRef, refLabel } from "../resolve/refs.js"; function sigil(action: PlanAction): string { switch (action) { @@ -19,7 +20,11 @@ function sigil(action: PlanAction): string { } function fmt(value: unknown): string { - return value === undefined ? "(none)" : JSON.stringify(value); + if (value === undefined) return "(none)"; + // A pending reference (#20): its target is created in this same apply, so its id is unknown until + // then. Render it like the permission scope pending marker instead of a raw sentinel object. + if (isPendingRef(value)) return `<${refLabel(value.__pendingRef)} (created this apply)>`; + return JSON.stringify(value); } export function renderPlan(plan: Plan): string { diff --git a/src/permissions/plan.ts b/src/permissions/plan.ts index 761631c..c74c982 100644 --- a/src/permissions/plan.ts +++ b/src/permissions/plan.ts @@ -12,6 +12,8 @@ import { resolveAuthId } from "./catalog.js"; import { resolveScope } from "./scope.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 } from "../resolve/refs.js"; export interface PermissionPlanItem { key: string; domainType: DomainType; domainId: number; diff: GrantDiff } @@ -57,16 +59,67 @@ export function desiredTuples( }); } +/** A permission whose domainId has been resolved from a logical Ref to a concrete numeric id. */ +type ResolvedPermission = DesiredPermission & { domainId: number }; + +/** + * Resolve every permission's domainId to a number (#20). A numeric domainId passes straight through; + * a Ref (e.g. `groupType: "…"`) resolves against the live catalog. A domainId that resolves to a + * same-run-created resource (PendingRef) is rejected — the permission plan needs a concrete id to + * fetch actuals and build the write path, and the permission subsystem does not defer that. A + * group_role ref throws its own gated "pass a numeric id" error from the resolver. + * + * After resolution, the authoritative duplicate-target guard runs on the CONCRETE ids: two different + * refs (or a ref and a number) that collide on one (domainType, domainId) would otherwise each diff + * against the other's grants and churn forever. Mirrors the eval-time guard in config/context.ts. + */ +async function resolveDomainIds( + permissions: DesiredPermission[], resolver: Resolver, +): Promise { + const resolved: ResolvedPermission[] = []; + for (const p of permissions) { + if (typeof p.domainId === "number") { + resolved.push(p as ResolvedPermission); + continue; + } + const site = `${p.domainType} "${p.key}".domainId`; + const res = await resolver.resolve(p.domainId, site); + if (isPendingRef(res)) { + throw new Error( + `${site}: references a resource created in the same run — apply it first, or use a numeric id.`, + ); + } + resolved.push({ ...p, domainId: res }); + } + const seen = new Map(); + for (const p of resolved) { + const key = `${p.domainType}:${p.domainId}`; + const prev = seen.get(key); + if (prev) { + throw new Error( + `Duplicate permission target after resolution: ${p.domainType} #${p.domainId} is declared by ` + + `both "${prev}" and "${p.key}". Merge their grants into one declaration.`, + ); + } + seen.set(key, p.key); + } + return resolved; +} + export async function buildPermissionPlan( client: Pick, state: State, permissions: DesiredPermission[], desired: DesiredResource[] = [], + resolver?: Resolver, ): Promise<{ items: PermissionPlanItem[]; fetchErrors: string[] }> { const items: PermissionPlanItem[] = []; const fetchErrors: string[] = []; + // Resolve logical domainIds (#20) up front. Shares the command layer's resolver so master-data + // catalogs are fetched once across buildPlan + buildPermissionPlan; falls back to a private one. + const resolved = await resolveDomainIds(permissions, resolver ?? new Resolver({ client, state, desired })); // Keys declared as groups in the config — valid scope targets even before they are created. const declaredGroupKeys = new Set(desired.filter((r) => r.type === "group").map((r) => r.key)); // one bulk fetch per distinct domainType const byType = new Map(); - for (const dt of new Set(permissions.map((p) => p.domainType))) { + for (const dt of new Set(resolved.map((p) => p.domainType))) { try { byType.set(dt, await client.get(`/permissions/${dt}`)); } catch (err) { @@ -75,7 +128,7 @@ export async function buildPermissionPlan( byType.set(dt, null); } } - for (const p of permissions) { + for (const p of resolved) { const all = byType.get(p.domainType); if (all == null) continue; // fetch failed for this domainType — recorded above const actual = normalizeActual(all.filter((r) => r.domainId === p.domainId)); diff --git a/src/permissions/types.ts b/src/permissions/types.ts index f4cb7b0..55be2d5 100644 --- a/src/permissions/types.ts +++ b/src/permissions/types.ts @@ -5,12 +5,18 @@ * are collected as a separate list from `DesiredResource[]`. */ import type { DomainType } from "./grants.js"; +import type { Ref } from "../resolve/refs.js"; export type Grant = string | { right: string; scope: string[] }; export interface DesiredPermission { key: string; domainType: DomainType; - domainId: number; + /** + * The permission domain. A raw number (escape hatch), or a logical {@link Ref} (#20) the per-host + * resolver turns into a numeric id at plan time (see `buildPermissionPlan`). Only ever a number + * downstream of resolution. + */ + domainId: number | Ref; grants: Grant[]; } diff --git a/src/resolve/refs.ts b/src/resolve/refs.ts new file mode 100644 index 0000000000000000000000000000000000000000..eff528b1ab7199c144a79c44a27cee76bb7e4ac4 GIT binary patch literal 6115 zcmbtY+in|265VHgMJ;0_LqQ}H1N%a)k`-*j@y5Bp7WTz3G|ibN+2L?z(%mDOD-*~^ z>^JO(3Sa!r5y{vU{2*{Z4eM8B&wEp}9@tv4OtppUHu?Iau45`4*8ZWR45R|zn0qb-27bzT9$_KaBcn@_K zD%&v30t(xZT;0Na0VKy9DBe2k*w&`dyt%W0b)2&n^v#rhf=xLUF>Ra|yaZrCX=t45 ztWZ{Yh9n0u&HPmGnA}D9&S}I*{r8CW<5bQi0_0Z8ayoHL*szJfD#!PNc|eKXn)>Rg zb_VbjWDz|UM38=07bZhkQmdEK)0Eb_07N7=O$mf(sw-z$D;Lz_ec|XNs?#e29~&g2 z3P>NqoKewPbcO7Eg&c!rVj(}nxIcdXub9Suo}HXPwaOyPIhA484+inNpiz)LqW3hC zk0-og@CbhKjguFg$CF$y`_=xh2``S$Pflj1r}WxvYXlG6Fj099WIDtOh6ZYzxAZX! z>IRkChait;!WxL;(v;#Zyb=ATY)rHTQPP1Sf<%}_cPB*G*O|xPbMm&)=XkM!vGB;} z*(wKVq(ea4`N=+5`vh368bN<&;Q=G2qWCp?D2?=pbm<%ererEQU`wi%vG<$$oJpC( z{b$yOaK^Y$@LsPxiG%xR7;}UWprd!|dm73`oR>|RNpe~JTZ2USLGO~_X6VJ>Itzq6 zZy9IY%90S7B_)isY8kfCUoQa0;8~#ReRw-9bh+|ty1?SsWBHIk2)4Qfv)s}%Yb%>b zsy(`6;rlsfpw7U5$eOY$Cwg1^U5r>LRx8xIM|b~x{qs-LAkHjzi6r@WKTXFRmX9F?=P$Ctjvj8kk zcl;d^CmmC5tGwY3;($2=3&aqEfKjrbJz~rW6B1)i2~xHV6XMl}|KeK$yBy@z*-=*E znLi%Dq0Kj08G~idAWBflDu1H1fNCMBdn6vOwo-$0K1*aI!Uu&8*ITiid{3E(KFP8C zweamdG7r~Ud1C%2N|SF#_lO!Sv~oc52qn<^{PODAQ)yLaUp)RBAj6pZkjc2PqxV=V zZZ|{uw_Lw3e|bTnZ8^iQd>lI_8_t(p=k_K;>uYILE5}cLS34rjByi9(g;LD)2wv3% zez>4(6u;2)K*tj40R3kzAYZ}N@LTj>;Dyk*4$|TIka;B|_>iNlkXNzZ$Z?@EGW0tR z9_Gk!Pe=|)la`(z+{yruot##C5OPfIj+c#t2LiDTIarvEJ;MIs+>bw4;UZIx$G~Y2*r_?oftfxqR)_0?h#$8+4#OH}mw8p(O@X zX1UIrnlU3C7$rQS@Oo|>8snWbPEPB#=LV0xR0~}|>YzF39D!vnPIk^FA6B7gyPa~t zMAg^?v3E!G9R-G!5Y8Xn+0a4m|H6OrmVHWdO8F^NANfn~_Gw!&==)}?$_ZNXT&ylw zPq8Jyc8lIft-#gXJxkM%Q7Fg6R<0hhMNIu{YXa0SdYb~;<%L)_{xj3KSY66&F~Mvx ziDL)BTo{CDQ`wdFfe;vKJUVjfc5_OoY1k=K3IG{r(?x@!1_9q`&myqO%!;uS1i^h0 zcm$gd#~5av?qF|thQeTDw42h8hWQ~B!@*?evZ|Irv<@>B^tKy(CsSL$&&_0`jBPwc zptBDFl8KweO3*lZP|9jVTIN5rA1fY_!~}Hwexl#$tnnDe*chfO8NK4nc>v~kL>%`1 z#Z&E`-4Nf)40%w^`!={A5GA#*ucLJcfS_Msc}x`jo*q?42gN+$RPT4A))R=QZydXf zGp_&sBMj7U8WK?=O!+Z7k_~~!xND4*ByR)h9gAwM)?oiQziiwe1n>Bj!$o^B0V)pS`VU>u~6q%`VP_l0M2BV9NF9+$RewdUTm z$cm+#U2msAXd8CWm3MYG;Jd;8qO?=#|bYoBzG7>Cu*N zx$^~jxhivMNx~OzHW*vp63+fvs^1hv%7RCFL_KgE`~YchHg9q6ho%HM(NWmP&kqe5 z^rR{Z6yAukA;2`QhjJ80W`hVzO1feR0%AhKHDaOdSB`$I7Ib6Oph}_)K2RgGAoCtv zApCgy=53f0WNwIV<(C)q<}Di&yeIi`f!?5l48Fdpz}N`zo%nhHAGj>MBX->s7m90O zfluRq!CB`tn|%O!xs#KNrBXY&>cW0@XF%r>?gnCRrU>;jG#LqNdMru~=#ROPk+ki1 zT0RO*LTrtC0|pR!H$aad7vSMO>0#ioGReEB4l5wIl(&6G4Z#yTsVm%WklX{dU_cn+ z&79|(Jj{4}&l(}r)9t>mcDM%ilg!wxL1|+vq%2M3D%5JexZ|ta{>r6e67=%@fk#|| zs4sB)tZ|3Q5e_jw-rX}tdrR|wl7_3cIyn&kIZP4#8q>WE7#H1-1#S9(5`&~(DiA%F Gl>Y(PnE$;1 literal 0 HcmV?d00001 diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts new file mode 100644 index 0000000..faad57a --- /dev/null +++ b/src/resolve/resolver.ts @@ -0,0 +1,236 @@ +/** + * The per-host reference resolver (#20). Turns logical {@link Ref}s into numeric + * ChurchTools ids, sourced (in order) from: + * + * 1. Managed desired ∪ state, by logical key. A key that names a managed resource + * in state resolves to its id; a key declared in this config but not yet in + * state resolves to a {@link PendingRef} (its id is only known after the + * resource tier applies — re-resolved at apply time, mirroring the permission + * scope pattern in src/permissions/scope.ts). + * 2. Live catalog master data, matched by `slug(name) === key` with an exact-name + * secondary: campus → /campuses, group-type → /group/grouptypes, + * group-status → /group/memberstatus, role-def → /group/roles. Each catalog is + * fetched at most once per run and cached by a `Map`, so the + * resolver is safe to share across `buildPlan` and `buildPermissionPlan` running + * concurrently (both await the same in-flight promise). + * 3. Hard error naming the kind, key, referencing site, and host. + * + * Unknown / ambiguous references THROW (a config error — distinct from the + * degrade-and-continue fetchErrors path). Resolved ids are never written back to + * config; only state carries ids. + */ +import type { CtClient } from "../api/ctClient.js"; +import type { State } from "../state/state.js"; +import type { DesiredResource } from "../engine/types.js"; +import { slug } from "../resources/registry.js"; +import { + collectRefs, + deepMapRefs, + isPendingRef, + isRef, + pendingRef, + refKey, + refLabel, + type GroupRoleRef, + type PendingRef, + type Ref, + type RefKind, + type SimpleRef, +} from "./refs.js"; + +/** ref kind → managed resource type (state/desired). group-status is read-only master data (catalog only). */ +const REF_KIND_TYPE: Partial> = { + campus: "campus", + "group-type": "group-type", + "role-def": "group-role", + group: "group", +}; + +/** ref kind → live catalog path. `group` has no catalog (managed-only); `group-role` is gated. */ +const CATALOG_PATH: Partial> = { + campus: "/campuses", + "group-type": "/group/grouptypes", + // Assumption (documented): /group/memberstatus rows carry a `name` field, like every other + // master-data catalog here (campus/grouptype/role all expose `name`). The endpoint is GET-only + // (docs/api-coverage.md #8), so this is matched, never written. If a live instance names the + // field differently, resolution falls through to the exact-name secondary and then a hard error. + "group-status": "/group/memberstatus", + "role-def": "/group/roles", +}; + +interface CatalogRecord { + id: number; + name?: string; + [k: string]: unknown; +} + +export interface ResolverDeps { + client: Pick; + state: State; + desired: DesiredResource[]; + /** Host label for error messages. Defaults to `state.host`. */ + host?: string; +} + +/** + * GATED (#20/#25): resolve a (group, role) pair to CT's internal group_role pairing domainId. + * TODO(#25): the candidate source is `GET /groups/{groupId}/roles` (per-group role assignments), + * but the pairing id is NOT confirmed to be exposed there — verify live on eqrm-dev before wiring + * this up. Until then the resolver rejects group_role refs with a clear "pass a numeric id" error; + * this seam exists so the lookup can be dropped in without touching call sites. + */ +export async function lookupGroupRolePairing( + groupId: number, + roleId: number, + client: Pick, +): Promise { + // Reference the seam's inputs so the intended call shape is documented in one place: + // const roles = await client.get(`/groups/${groupId}/roles`); find the row for `roleId`; its + // pairing id is the group_role domainId — IF the endpoint exposes it (unconfirmed). + void client; + throw new Error( + `group_role (group ${groupId}, role ${roleId}) → domainId lookup is not implemented (#25).`, + ); +} + +export class Resolver { + private readonly client: Pick; + private readonly state: State; + private readonly host: string; + private readonly catalogs = new Map>(); + /** Declared logical keys indexed by resource type — a same-run target that resolves to pending. */ + private readonly declaredByType = new Map>(); + + constructor(deps: ResolverDeps) { + this.client = deps.client; + this.state = deps.state; + this.host = deps.host ?? deps.state.host; + for (const d of deps.desired) { + let set = this.declaredByType.get(d.type); + if (!set) { + set = new Set(); + this.declaredByType.set(d.type, set); + } + set.add(d.key); + } + } + + /** Resolve one Ref to a numeric id, or a {@link PendingRef} for a same-run-created managed target. */ + async resolve(r: Ref, site: string): Promise { + if (r.kind === "group-role") return this.resolveGroupRole(r, site); + // (1) managed desired ∪ state by logical key + const type = REF_KIND_TYPE[r.kind]; + if (type !== undefined) { + const managed = this.state.resources[r.key]; + if (managed && managed.type === type) return managed.id; + if (this.declaredByType.get(type)?.has(r.key)) return pendingRef(r); + } + // (2) live catalog + if (CATALOG_PATH[r.kind] !== undefined) return this.resolveFromCatalog(r, site); + // (3) hard error + throw this.notFound(r, site); + } + + /** + * Deep-rewrite every Ref embedded in `value` to its resolved id / pending marker. Returns the + * original reference unchanged when it holds no Refs (numbers pass through untouched — the numeric + * escape hatch — so a fully-numeric field bag is never rebuilt and diffs number↔number). + */ + async resolveValue(value: unknown, site: string): Promise { + const refs = collectRefs(value); + if (refs.length === 0) return value; + const byKey = new Map(); + for (const r of refs) { + const k = refKey(r); + if (!byKey.has(k)) byKey.set(k, await this.resolve(r, site)); + } + return deepMapRefs(value, (r) => byKey.get(refKey(r))); + } + + private catalog(kind: RefKind): Promise { + let p = this.catalogs.get(kind); + if (!p) { + const path = CATALOG_PATH[kind]!; + p = this.client.get(path).then((rows) => (Array.isArray(rows) ? rows : [])); + this.catalogs.set(kind, p); + } + return p; + } + + private async resolveFromCatalog(r: SimpleRef, site: string): Promise { + const rows = await this.catalog(r.kind); + const pick = (candidates: CatalogRecord[]): number => { + if (candidates.length > 1) throw this.ambiguous(r, site, candidates); + return candidates[0]!.id; + }; + // Primary: slugified name. Secondary: exact (case-sensitive) name — covers a name that does not + // survive slugging cleanly. Ambiguity in either bucket is a hard error listing the candidates. + const bySlug = rows.filter((row) => typeof row.name === "string" && slug(row.name) === r.key); + if (bySlug.length >= 1) return pick(bySlug); + const byExact = rows.filter((row) => row.name === r.key); + if (byExact.length >= 1) return pick(byExact); + throw this.notFound(r, site); + } + + private resolveGroupRole(r: GroupRoleRef, site: string): never { + throw new Error( + `Cannot resolve ${refLabel(r)} referenced at ${site} on ${this.host}: resolving a ` + + `(group, role) pair to its permission domainId is not yet supported — pass a numeric id ` + + `instead (see #25).`, + ); + } + + private notFound(r: SimpleRef, site: string): Error { + const catalog = CATALOG_PATH[r.kind]; + const where = catalog + ? `no managed resource and no live ${r.kind} at ${catalog} matches key "${r.key}"` + : `no managed ${r.kind} named "${r.key}" is declared or adopted`; + return new Error( + `Cannot resolve ${refLabel(r)} referenced at ${site} on ${this.host}: ${where}. ` + + `Declare/adopt it, fix the key/name, or use a numeric id.`, + ); + } + + private ambiguous(r: SimpleRef, site: string, candidates: CatalogRecord[]): Error { + const list = candidates.map((c) => `${JSON.stringify(c.name)} (#${c.id})`).join(", "); + return new Error( + `Ambiguous ${refLabel(r)} referenced at ${site} on ${this.host}: ${candidates.length} live ` + + `${r.kind}s match — ${list}. Rename to disambiguate, or use a numeric id.`, + ); + } +} + +/** + * Re-resolve every {@link PendingRef} in a value against the POST-execute state, at apply time. + * The referenced resource's tier applies before the referencing resource's (campus tier 0 < group + * tier 1), so its id is guaranteed present in state by the time the referencing body is built — + * analogous to the permission scope `reresolveTuple`. Passes non-pending values through untouched. + */ +export function reresolvePendingValue(value: unknown, state: State): unknown { + if (isPendingRef(value)) return pendingIdFromState(value.__pendingRef, state); + if (Array.isArray(value)) return value.map((v) => reresolvePendingValue(v, state)); + if (value !== null && typeof value === "object") { + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) out[k] = reresolvePendingValue(v, state); + return out; + } + return value; +} + +function pendingIdFromState(r: Ref, state: State): number { + if (r.kind === "group-role") { + // group_role refs are gated at plan time, so a pending one should never reach apply. + throw new Error(`Pending ${refLabel(r)} reached apply — group_role refs are unsupported (#25).`); + } + const managed = state.resources[r.key]; + if (!managed) { + throw new Error( + `Pending reference ${refLabel(r)} did not resolve after apply — "${r.key}" is not in state. ` + + `Its tier should have applied first.`, + ); + } + return managed.id; +} + +/** Re-export for callers that only need the guard without importing refs.ts directly. */ +export { isRef }; diff --git a/tests/context.test.ts b/tests/context.test.ts index 434aa1d..d9de134 100644 --- a/tests/context.test.ts +++ b/tests/context.test.ts @@ -78,9 +78,27 @@ describe("config context", () => { expect(r2[0]?.fields).toEqual({ name: "Team", campusId: null }); }); - it("rejects a logical `campus` reference (deferred to #20) and a non-numeric campusId", () => { + it("sugars a logical `campus`/`groupType`/`status` into a Ref-valued id field (#20)", () => { + const { ct, resources } = createContext(); + ct.group({ key: "g", name: "G", campus: "mainz", groupType: "ministry_team", status: "active" }); + expect(resources[0]?.fields).toEqual({ + name: "G", + campusId: { __ctRef: true, kind: "campus", key: "mainz" }, + groupTypeId: { __ctRef: true, kind: "group-type", key: "ministry_team" }, + groupStatusId: { __ctRef: true, kind: "group-status", key: "active" }, + }); + }); + + it("rejects declaring both the logical and the numeric id form", () => { + const { ct } = createContext(); + expect(() => ct.group({ key: "g", name: "G", campus: "mainz", campusId: 4 })).toThrow( + /either "campus".*or "campusId".*not both/, + ); + }); + + it("rejects a non-string logical reference and a non-numeric/non-ref id field", () => { const { ct } = createContext(); - expect(() => ct.group({ key: "g", name: "G", campus: "mainz" })).toThrow(/not supported yet/); + expect(() => ct.group({ key: "g", name: "G", campus: 4 as never })).toThrow(/non-empty string key/); expect(() => ct.group({ key: "h", name: "H", campusId: "4" as never })).toThrow(/must be a number/); }); @@ -235,7 +253,7 @@ describe("permission declarations", () => { it("rejects a non-numeric id and an empty right name", async () => { await expect( evaluateConfig((ct: ConfigContext) => ct.groupRole({ key: "x", id: "nope", grants: [] } as never)), - ).rejects.toThrow(/id.*number/i); + ).rejects.toThrow(/numeric "id"/i); await expect( evaluateConfig((ct: ConfigContext) => ct.groupRole({ key: "x", id: 1, grants: [""] })), ).rejects.toThrow(/grant/i); @@ -244,7 +262,7 @@ describe("permission declarations", () => { it("rejects a non-finite id (NaN)", async () => { await expect( evaluateConfig((ct: ConfigContext) => ct.groupRole({ key: "x", id: NaN, grants: [] })), - ).rejects.toThrow(/id.*number/i); + ).rejects.toThrow(/numeric "id"/i); }); it("rejects two declarations targeting the same (domainType, domainId)", async () => { From 4ce7e83d923b73436bc4ec3bf3880436cab8e23d Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 9 Jul 2026 09:02:52 +0200 Subject: [PATCH 2/5] test(refs): unit + end-to-end coverage for portable references (#20) refs.ts (helper/guards/deepMapRefs/collectRefs), resolver.ts (state/catalog/ pending/ambiguous/unknown/gated group_role + resolveValue caching + apply-time re-resolution), DSL permission logical forms, and the two-host acceptance test. --- src/resolve/refs.ts | Bin 6115 -> 6115 bytes tests/context.test.ts | 31 +++++++ tests/portable-refs.test.ts | 178 ++++++++++++++++++++++++++++++++++++ tests/refs.test.ts | 100 ++++++++++++++++++++ tests/resolver.test.ts | 147 +++++++++++++++++++++++++++++ 5 files changed, 456 insertions(+) create mode 100644 tests/portable-refs.test.ts create mode 100644 tests/refs.test.ts create mode 100644 tests/resolver.test.ts diff --git a/src/resolve/refs.ts b/src/resolve/refs.ts index eff528b1ab7199c144a79c44a27cee76bb7e4ac4..a5bae913da418559bbd95fcbc437883ef88937d2 100644 GIT binary patch delta 14 VcmaE?|5$&+R$fMh&D(f&xd1TI1!VvL delta 14 VcmaE?|5$&+R$fMi&D(f&xd1Q{1w{Y= diff --git a/tests/context.test.ts b/tests/context.test.ts index d9de134..4d24976 100644 --- a/tests/context.test.ts +++ b/tests/context.test.ts @@ -281,4 +281,35 @@ describe("permission declarations", () => { }); expect(permissions).toHaveLength(2); }); + + it("sugars a logical `groupType` into a Ref-valued domainId (#20)", async () => { + const { permissions } = await evaluateConfig((ct: ConfigContext) => + ct.groupTypeRole({ key: "tpl", groupType: "ministry_team", grants: ["churchgroup:view group"] }), + ); + expect(permissions[0]?.domainId).toEqual({ __ctRef: true, kind: "group-type", key: "ministry_team" }); + }); + + it("sugars group_role `group` + `role` into a compound Ref (gated at plan time, #25)", async () => { + const { permissions } = await evaluateConfig((ct: ConfigContext) => + ct.groupRole({ key: "p", group: "kids", role: "Leiter", grants: ["churchgroup:view group"] }), + ); + expect(permissions[0]?.domainId).toEqual({ __ctRef: true, kind: "group-role", group: "kids", role: "Leiter" }); + }); + + it("rejects declaring both a numeric id and a logical domain form", async () => { + await expect( + evaluateConfig((ct: ConfigContext) => + ct.groupTypeRole({ key: "tpl", id: 8, groupType: "mt", grants: ["churchgroup:view group"] }), + ), + ).rejects.toThrow(/either "id".*or "groupType".*not both/); + }); + + it("dedups two logical declarations targeting the same group-type ref", async () => { + await expect( + evaluateConfig((ct: ConfigContext) => { + ct.groupTypeRole({ key: "a", groupType: "mt", grants: ["churchgroup:view group"] }); + ct.groupTypeRole({ key: "b", groupType: "mt", grants: ["churchdb:view group members"] }); + }), + ).rejects.toThrow(/Duplicate permission target.*group-type:mt/s); + }); }); diff --git a/tests/portable-refs.test.ts b/tests/portable-refs.test.ts new file mode 100644 index 0000000..89da130 --- /dev/null +++ b/tests/portable-refs.test.ts @@ -0,0 +1,178 @@ +/** + * End-to-end coverage for portable logical references (#20): the resolver wired through buildPlan, + * apply-time pending re-resolution in executePlan, permission domainId resolution, and the headline + * acceptance test — one config yielding valid plans against two different hosts (states + catalogs). + */ +import { describe, it, expect } from "vitest"; +import { evaluateConfig } from "../src/config/context.js"; +import { buildPlan } from "../src/engine/build.js"; +import { executePlan } from "../src/engine/execute.js"; +import { buildPermissionPlan } from "../src/permissions/plan.js"; +import { Resolver } from "../src/resolve/resolver.js"; +import { renderPlan } from "../src/engine/render.js"; +import { CtApiError } from "../src/api/ctClient.js"; +import { emptyState, type State } from "../src/state/state.js"; + +/** Fake client serving catalog/item GETs and recording write requests, returning canned POST ids. */ +function fakeHost(catalogs: Record, postIds: Record = {}) { + const calls: { method: string; path: string; body?: unknown }[] = []; + return { + calls, + get: async (path: string): Promise => { + if (!(path in catalogs)) throw new CtApiError(`not found: ${path}`, 404, null); + return catalogs[path] as T; + }, + request: async (method: string, path: string, body?: unknown): Promise => { + calls.push({ method, path, body }); + const id = postIds[`${method} ${path}`]; + return (id !== undefined ? { id } : {}) as T; + }, + }; +} + +const noSave = async (): Promise => {}; + +describe("buildPlan reference resolution", () => { + it("resolves a catalog groupType ref to a number so the diff stays number↔number", async () => { + const { resources } = await evaluateConfig((ct) => { + ct.group({ key: "kids", name: "Kids", groupType: "ministry_team" }); + }); + const client = fakeHost({ "/group/grouptypes": [{ id: 2, name: "Ministry Team" }] }); + const { plan } = await buildPlan(client, emptyState("h"), resources); + const item = plan.items.find((i) => i.key === "kids")!; + expect(item.action).toBe("create"); + expect(item.changes).toContainEqual({ field: "groupTypeId", from: undefined, to: 2 }); + }); + + it("renders a same-run campus reference as a pending marker", async () => { + const { resources } = await evaluateConfig((ct) => { + ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); + ct.group({ key: "kids", name: "Kids", groupTypeId: 2, campus: "mainz" }); + }); + const client = fakeHost({}); + const { plan } = await buildPlan(client, emptyState("h"), resources); + const rendered = renderPlan(plan); + expect(rendered).toContain("campusId: "); + }); + + it("throws a config error (not a fetchError) on an unresolvable reference", async () => { + const { resources } = await evaluateConfig((ct) => { + ct.group({ key: "kids", name: "Kids", groupType: "ghost_type" }); + }); + const client = fakeHost({ "/group/grouptypes": [{ id: 2, name: "Ministry Team" }] }); + await expect(buildPlan(client, emptyState("h"), resources)).rejects.toThrow( + /Cannot resolve group-type:ghost_type referenced at group "kids"/, + ); + }); +}); + +describe("apply-time pending re-resolution (same-run campus + group)", () => { + it("carries the freshly-created campus id into the group's POST body", async () => { + const { resources } = await evaluateConfig((ct) => { + ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); + ct.group({ key: "kids", name: "Kids", groupTypeId: 2, campus: "mainz" }); + }); + const state = emptyState("h"); + const host = fakeHost({}, { "POST /campuses": 42, "POST /groups": 100 }); + const { plan } = await buildPlan(host, state, resources); + + await executePlan(plan, { client: host, state, statePath: "s.json", save: noSave, now: () => "t" }); + + const campusPost = host.calls.find((c) => c.path === "/campuses")!; + expect(campusPost.method).toBe("POST"); + const groupPost = host.calls.find((c) => c.path === "/groups")!; + // The group POST body carries the campus's freshly created id (42), not the pending sentinel. + expect(groupPost.body).toEqual({ name: "Kids", groupTypeId: 2, campusId: 42 }); + // State records the resolved id too (no pending marker leaks into state). + expect(state.resources.kids?.fields).toMatchObject({ campusId: 42 }); + }); +}); + +describe("permission domainId resolution", () => { + it("resolves a groupType ref to the domainId and diffs against it", async () => { + const { permissions } = await evaluateConfig((ct) => { + ct.groupTypeRole({ key: "tpl", groupType: "ministry_team", grants: ["churchgroup:administer groups"] }); + }); + const client = { + get: async (path: string): Promise => { + if (path === "/group/grouptypes") return [{ id: 9, name: "Ministry Team" }] as T; + if (path === "/permissions/group_type_role") return [] as T; + throw new CtApiError(`not found: ${path}`, 404, null); + }, + }; + const { items } = await buildPermissionPlan(client, emptyState("h"), permissions); + expect(items).toHaveLength(1); + expect(items[0]?.domainId).toBe(9); // resolved from the catalog, not a raw number + }); + + it("rejects two permissions whose refs resolve to the same domainId (post-resolution guard)", async () => { + const { permissions } = await evaluateConfig((ct) => { + ct.groupTypeRole({ key: "a", groupType: "ministry_team", grants: ["churchgroup:administer groups"] }); + ct.groupTypeRole({ key: "b", id: 9, grants: ["churchgroup:administer groups"] }); + }); + const client = { + get: async (path: string): Promise => { + if (path === "/group/grouptypes") return [{ id: 9, name: "Ministry Team" }] as T; + if (path === "/permissions/group_type_role") return [] as T; + throw new CtApiError(`not found: ${path}`, 404, null); + }, + }; + await expect(buildPermissionPlan(client, emptyState("h"), permissions)).rejects.toThrow( + /Duplicate permission target after resolution: group_type_role #9/, + ); + }); + + it("rejects a gated group_role reference at plan time", async () => { + const { permissions } = await evaluateConfig((ct) => { + ct.groupRole({ key: "p", group: "kids", role: "Leiter", grants: ["churchgroup:administer groups"] }); + }); + const client = { get: async (): Promise => [] as T }; + await expect(buildPermissionPlan(client, emptyState("h"), permissions)).rejects.toThrow( + /not yet supported.*pass a numeric id.*#25/, + ); + }); +}); + +describe("acceptance: one config, two hosts", () => { + /** The identical config module — no numeric ids anywhere the resolver can fill in per host. */ + const config = (ct: Parameters[0]>[0]): void => { + ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); + ct.group({ key: "kids", name: "Kids", groupType: "ministry_team", campus: "mainz" }); + ct.groupTypeRole({ + key: "tpl", + groupType: "ministry_team", + grants: [{ right: "churchgroup:view group", scope: ["kids"] }], + }); + }; + + async function planFor(groupTypeId: number, state: State) { + const { resources, permissions } = await evaluateConfig(config); + const catalogs = { + "/group/grouptypes": [{ id: groupTypeId, name: "Ministry Team" }], + "/permissions/group_type_role": [], + }; + const client = fakeHost(catalogs); + const resolver = new Resolver({ client, state, desired: resources, host: state.host }); + const { plan } = await buildPlan(client, state, resources, { resolver }); + const { items } = await buildPermissionPlan(client, state, permissions, resources, resolver); + return { plan, items }; + } + + it("produces valid, host-specific plans against two different catalogs without editing the config", async () => { + // Host A: group type id 2. Host B: the SAME group type named differently in the catalog → id 77. + const a = await planFor(2, emptyState("https://a.church.tools")); + const b = await planFor(77, emptyState("https://b.church.tools")); + + const groupOf = (p: typeof a.plan) => p.items.find((i) => i.key === "kids")!; + expect(groupOf(a.plan).changes).toContainEqual({ field: "groupTypeId", from: undefined, to: 2 }); + expect(groupOf(b.plan).changes).toContainEqual({ field: "groupTypeId", from: undefined, to: 77 }); + + // Permission domainId is resolved per host from the same logical ref. + expect(a.items[0]?.domainId).toBe(2); + expect(b.items[0]?.domainId).toBe(77); + + // Both plans create the campus + group (2 creates each) — the config is valid against both hosts. + expect(a.plan.items.filter((i) => i.action === "create")).toHaveLength(2); + expect(b.plan.items.filter((i) => i.action === "create")).toHaveLength(2); + }); +}); diff --git a/tests/refs.test.ts b/tests/refs.test.ts new file mode 100644 index 0000000..8c42d9b --- /dev/null +++ b/tests/refs.test.ts @@ -0,0 +1,100 @@ +import { describe, it, expect } from "vitest"; +import { + ref, + isRef, + isPendingRef, + pendingRef, + refKey, + refLabel, + deepMapRefs, + collectRefs, + hasPendingRef, +} from "../src/resolve/refs.js"; + +describe("ref helper", () => { + it("builds the expected sentinels", () => { + expect(ref.campus("mainz")).toEqual({ __ctRef: true, kind: "campus", key: "mainz" }); + expect(ref.groupType("mt")).toEqual({ __ctRef: true, kind: "group-type", key: "mt" }); + expect(ref.status("active")).toEqual({ __ctRef: true, kind: "group-status", key: "active" }); + expect(ref.roleDef("leiter")).toEqual({ __ctRef: true, kind: "role-def", key: "leiter" }); + expect(ref.group("g")).toEqual({ __ctRef: true, kind: "group", key: "g" }); + expect(ref.groupRole("g", "Leiter")).toEqual({ __ctRef: true, kind: "group-role", group: "g", role: "Leiter" }); + }); + + it("rejects an empty/non-string key", () => { + expect(() => ref.campus("")).toThrow(/non-empty string/); + expect(() => ref.campus(4 as never)).toThrow(/non-empty string/); + expect(() => ref.groupRole("g", "")).toThrow(/non-empty string/); + }); +}); + +describe("isRef", () => { + it("recognises refs and rejects plain values", () => { + expect(isRef(ref.campus("x"))).toBe(true); + expect(isRef({ kind: "campus", key: "x" })).toBe(false); // missing __ctRef + expect(isRef(4)).toBe(false); + expect(isRef(null)).toBe(false); + expect(isRef("campus")).toBe(false); + }); +}); + +describe("refKey / refLabel", () => { + it("keys simple and compound refs distinctly", () => { + expect(refKey(ref.campus("mainz"))).toBe("campus:mainz"); + expect(refKey(ref.groupRole("g", "r"))).toBe("group-role:g r"); + expect(refLabel(ref.groupType("mt"))).toBe("group-type:mt"); + expect(refLabel(ref.groupRole("g", "r"))).toBe("group-role(group=g, role=r)"); + }); +}); + +describe("pendingRef / isPendingRef", () => { + it("wraps and recognises a pending marker", () => { + const p = pendingRef(ref.campus("mainz")); + expect(p).toEqual({ __pendingRef: { __ctRef: true, kind: "campus", key: "mainz" } }); + expect(isPendingRef(p)).toBe(true); + expect(isPendingRef(ref.campus("mainz"))).toBe(false); // a bare ref is not pending + expect(isPendingRef({ __pendingRef: 4 })).toBe(false); // must wrap a real ref + expect(isPendingRef(4)).toBe(false); + }); +}); + +describe("deepMapRefs", () => { + it("replaces refs anywhere in a nested structure, passing scalars through", () => { + const input = { + campusId: ref.campus("mainz"), + query: { "==": [{ var: "ctgroup.campusId" }, ref.campus("berlin")], n: 5, s: "x" }, + list: [1, ref.groupType("mt"), "keep"], + }; + const out = deepMapRefs(input, (r) => refKey(r)); + expect(out).toEqual({ + campusId: "campus:mainz", + query: { "==": [{ var: "ctgroup.campusId" }, "campus:berlin"], n: 5, s: "x" }, + list: [1, "group-type:mt", "keep"], + }); + }); + + it("returns scalars untouched", () => { + expect(deepMapRefs(5, () => 0)).toBe(5); + expect(deepMapRefs("x", () => 0)).toBe("x"); + expect(deepMapRefs(null, () => 0)).toBe(null); + }); +}); + +describe("collectRefs", () => { + it("gathers every ref in order, treating refs as leaves", () => { + const refs = collectRefs({ a: ref.campus("x"), b: [ref.groupType("y"), { c: ref.group("z") }], d: 4 }); + expect(refs.map(refKey)).toEqual(["campus:x", "group-type:y", "group:z"]); + }); + + it("returns [] when there are no refs (the numeric escape hatch)", () => { + expect(collectRefs({ campusId: 4, groupTypeId: 2 })).toEqual([]); + }); +}); + +describe("hasPendingRef", () => { + it("detects a pending marker at any depth", () => { + expect(hasPendingRef({ campusId: pendingRef(ref.campus("x")) })).toBe(true); + expect(hasPendingRef({ a: [1, { b: pendingRef(ref.group("g")) }] })).toBe(true); + expect(hasPendingRef({ campusId: 4, groupTypeId: 2 })).toBe(false); + }); +}); diff --git a/tests/resolver.test.ts b/tests/resolver.test.ts new file mode 100644 index 0000000..02535c2 --- /dev/null +++ b/tests/resolver.test.ts @@ -0,0 +1,147 @@ +import { describe, it, expect } from "vitest"; +import { Resolver, reresolvePendingValue } from "../src/resolve/resolver.js"; +import { ref, isPendingRef } from "../src/resolve/refs.js"; +import { CtApiError } from "../src/api/ctClient.js"; +import { emptyState, type State } from "../src/state/state.js"; +import type { DesiredResource } from "../src/engine/types.js"; + +/** A fake client returning canned catalogs by path; 404s on a miss. Counts GET calls per path. */ +function fakeClient(byPath: Record) { + const calls: Record = {}; + return { + calls, + get: async (path: string): Promise => { + calls[path] = (calls[path] ?? 0) + 1; + if (!(path in byPath)) throw new CtApiError(`not found: ${path}`, 404, null); + return byPath[path] as T; + }, + }; +} + +function stateWith(resources: State["resources"]): State { + return { ...emptyState("https://x.church.tools"), resources }; +} + +const NO_DESIRED: DesiredResource[] = []; + +describe("Resolver.resolve", () => { + it("resolves a campus from managed state by logical key (before any catalog fetch)", async () => { + const state = stateWith({ + mainz: { type: "campus", id: 7, key: "mainz", fields: {}, adoptedAt: "t", updatedAt: "t" }, + }); + const client = fakeClient({}); + const r = new Resolver({ client, state, desired: NO_DESIRED }); + expect(await r.resolve(ref.campus("mainz"), "site")).toBe(7); + expect(client.calls).toEqual({}); // state hit, no /campuses fetch + }); + + it("resolves a campus from the live catalog by slug(name)", async () => { + const client = fakeClient({ "/campuses": [{ id: 3, name: "Berlin", shorty: "BE" }, { id: 5, name: "Mainz" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + expect(await r.resolve(ref.campus("mainz"), "site")).toBe(5); + }); + + it("resolves a group type from the live catalog", async () => { + const client = fakeClient({ "/group/grouptypes": [{ id: 2, name: "Ministry Team" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + expect(await r.resolve(ref.groupType("ministry_team"), "site")).toBe(2); + }); + + it("resolves a group status from /group/memberstatus", async () => { + const client = fakeClient({ "/group/memberstatus": [{ id: 1, name: "Active" }, { id: 2, name: "Candidate" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + expect(await r.resolve(ref.status("candidate"), "site")).toBe(2); + }); + + it("returns a pending marker for a same-run-declared managed target (not yet in state)", async () => { + const desired: DesiredResource[] = [{ type: "campus", key: "mainz", fields: {}, dependsOn: [] }]; + const client = fakeClient({ "/campuses": [{ id: 99, name: "Mainz" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired }); + const res = await r.resolve(ref.campus("mainz"), "site"); + expect(isPendingRef(res)).toBe(true); + expect(client.calls).toEqual({}); // desired hit wins over the catalog + }); + + it("throws listing candidates on an ambiguous catalog match", async () => { + const client = fakeClient({ "/campuses": [{ id: 1, name: "Mainz" }, { id: 2, name: "Mainz" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED, host: "hostA" }); + await expect(r.resolve(ref.campus("mainz"), "group \"g\"")).rejects.toThrow( + /Ambiguous campus:mainz referenced at group "g" on hostA: 2 live campuss match/, + ); + }); + + it("throws a clear error on an unknown reference (kind + key + site + host)", async () => { + const client = fakeClient({ "/campuses": [{ id: 1, name: "Berlin" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED, host: "hostB" }); + await expect(r.resolve(ref.campus("mainz"), "group \"g\".campusId")).rejects.toThrow( + /Cannot resolve campus:mainz referenced at group "g".campusId on hostB/, + ); + }); + + it("throws the gated error for a group_role reference", async () => { + const client = fakeClient({}); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + await expect(r.resolve(ref.groupRole("g", "Leiter"), "perm \"p\"")).rejects.toThrow( + /not yet supported.*pass a numeric id.*#25/, + ); + }); + + it("errors on a group ref with no managed match (groups have no catalog)", async () => { + const client = fakeClient({}); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + await expect(r.resolve(ref.group("ghost"), "site")).rejects.toThrow(/no managed group named "ghost"/); + }); + + it("falls back to an exact-name secondary match when the slug misses", async () => { + const client = fakeClient({ "/group/grouptypes": [{ id: 8, name: "K-9" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + // slug("K-9") === "k_9", so ref.groupType("k_9") hits the slug path; "K-9" hits the exact path. + expect(await r.resolve(ref.groupType("K-9"), "site")).toBe(8); + }); +}); + +describe("Resolver.resolveValue", () => { + it("deep-rewrites refs to ids and fetches each catalog at most once", async () => { + const client = fakeClient({ "/campuses": [{ id: 5, name: "Mainz" }, { id: 6, name: "Berlin" }] }); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + const value = { + campusId: ref.campus("mainz"), + query: { or: [{ "==": [{ var: "ctgroup.campusId" }, ref.campus("mainz")] }, { "==": [{ var: "ctgroup.campusId" }, ref.campus("berlin")] }] }, + untouched: 42, + }; + const out = await r.resolveValue(value, "site"); + expect(out).toEqual({ + campusId: 5, + query: { or: [{ "==": [{ var: "ctgroup.campusId" }, 5] }, { "==": [{ var: "ctgroup.campusId" }, 6] }] }, + untouched: 42, + }); + expect(client.calls["/campuses"]).toBe(1); // cached across the two mainz refs + the berlin ref + }); + + it("returns the original reference untouched when there are no refs", async () => { + const client = fakeClient({}); + const r = new Resolver({ client, state: emptyState("h"), desired: NO_DESIRED }); + const value = { campusId: 4, groupTypeId: 2 }; + expect(await r.resolveValue(value, "site")).toBe(value); // identity — no rebuild, no fetch + expect(client.calls).toEqual({}); + }); +}); + +describe("reresolvePendingValue", () => { + it("replaces a pending marker with the id from post-execute state", () => { + const state = stateWith({ + mainz: { type: "campus", id: 12, key: "mainz", fields: {}, adoptedAt: "t", updatedAt: "t" }, + }); + const body = { name: "G", campusId: { __pendingRef: ref.campus("mainz") } }; + expect(reresolvePendingValue(body, state)).toEqual({ name: "G", campusId: 12 }); + }); + + it("throws if a pending target never landed in state", () => { + const body = { campusId: { __pendingRef: ref.campus("mainz") } }; + expect(() => reresolvePendingValue(body, emptyState("h"))).toThrow(/did not resolve after apply/); + }); + + it("passes non-pending values through untouched", () => { + expect(reresolvePendingValue({ campusId: 4, n: [1, 2] }, emptyState("h"))).toEqual({ campusId: 4, n: [1, 2] }); + }); +}); From b55ff75856cf3d9b6e1e1dd9a34f6258e30257a0 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 9 Jul 2026 09:09:11 +0200 Subject: [PATCH 3/5] docs(refs): logical-ref forms in examples, docs, README, runbook (#20) Convert examples to the named/ref forms (numeric escape-hatch notes kept); add examples/portable.config.ts (zero numeric ids). Update permissions.md, dynamic-groups.md, blueprints.md, README; mark #20 shipped in the runbook and note the gated group_role part (#25). --- README.md | 25 +++++++++---- docs/blueprints.md | 37 +++++++++++++------ docs/dynamic-groups.md | 15 +++++--- docs/permissions.md | 46 +++++++++++++++-------- docs/runbook-manual-surface.md | 14 +++---- examples/campus-blueprint.config.ts | 27 ++++++++------ examples/dynamic-group.config.ts | 14 +++---- examples/permissions.config.ts | 16 ++++---- examples/portable.config.ts | 57 +++++++++++++++++++++++++++++ 9 files changed, 180 insertions(+), 71 deletions(-) create mode 100644 examples/portable.config.ts diff --git a/README.md b/README.md index 08a97ad..bc789af 100644 --- a/README.md +++ b/README.md @@ -112,18 +112,29 @@ default-exports a function receiving the DSL: ```ts export default (ct) => { ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); - ct.group({ key: "mainz_area", name: "Mainz · Bereiche", groupTypeId: 2 }); + // Reference master data BY NAME, not by hardcoded id: `groupType: "…"` resolves to the + // per-host group-type id at plan time, so this config is portable across instances (#20). + ct.group({ key: "mainz_area", name: "Mainz · Bereiche", groupType: "ministry_team" }); // Hierarchy is opt-in and multi-parent: `parents` are managed group keys, each declared // in this config. Omit it to leave a group's hierarchy unmanaged; edges to unmanaged // groups stay invisible. (`parent:` is unrelated — an ordering hint only, not hierarchy.) - ct.group({ key: "mainz_kids_lead", name: "Mainz · Kids Leitung", groupTypeId: 2, parents: ["mainz_area"] }); - // Assign a group to a campus by its numeric id (CT stores it at `information.campusId`). - // `campusId: null` clears the assignment. A *logical* `campus: "mainz"` reference — resolving a - // same-run campus by key — is deferred to #20; use the existing campus's numeric id for now. - ct.group({ key: "mainz_kids", name: "Mainz · Kids", groupTypeId: 2, campusId: 3, parents: ["mainz_kids_lead"] }); + ct.group({ key: "mainz_kids_lead", name: "Mainz · Kids Leitung", groupType: "ministry_team", parents: ["mainz_area"] }); + // Assign a group to a campus BY KEY: `campus: "mainz"` links to the campus above even though + // it is created in the same apply (its id is filled in at apply time). The numeric escape + // hatch still works — `campusId: 3` (or `campusId: null` to clear) targets an existing id. + ct.group({ key: "mainz_kids", name: "Mainz · Kids", groupType: "ministry_team", campus: "mainz", parents: ["mainz_kids_lead"] }); }; ``` +**Portable references (#20):** logical fields (`campus`/`groupType`/`status` on a +group, `groupType` on a permission) and the inline `ref.*` helper compile to id-free +sentinels a per-host resolver maps to real ChurchTools ids at plan time — sourced from +resources this tool manages, then live master-data catalogs matched by name. So one +config file plans and applies unchanged against different instances (ids differ per +host). An unresolvable name fails the plan with a clear error naming the reference and +where it was used. Raw numeric ids remain a valid escape hatch everywhere; see +[`examples/portable.config.ts`](examples/portable.config.ts) for a zero-numeric-id config. + `campusId` is a managed group field: `ct plan` shows a campus assign/move/clear as a normal field update, and `ct adopt group ` captures it. Which group fields are managed vs. deliberately left to the CT UI is recorded in @@ -176,7 +187,7 @@ runnable example. ```ts ct.groupTypeRole({ key: "leiter_tpl", - id: 1, // the domainId — the group type's own id for group_type_role + groupType: "kids", // domain BY NAME — resolved to the group-type domainId per host (#20) grants: [ "churchgroup:view", // unscoped { right: "churchgroup:view group", scope: ["kids_area"] }, // scoped to a managed group diff --git a/docs/blueprints.md b/docs/blueprints.md index a87e85d..e4b687f 100644 --- a/docs/blueprints.md +++ b/docs/blueprints.md @@ -36,18 +36,31 @@ language, no generated files, just a function called twice. ### Assigning the blueprint's groups to their campus -A group is linked to a campus with a numeric `campusId: ` -(CT stores it at `information.campusId`; `campusId: null` clears it). `ct plan` -diffs a campus assign/move/clear as a normal field update — see -[`docs/group-field-decisions.md`](group-field-decisions.md). - -The catch for a per-campus blueprint: the blueprint usually *creates* the campus -in the same apply, and a numeric id doesn't exist until after that create. So -linking a group to a **same-run** campus by key (`campus: "mainz"`) needs the -logical-reference resolver and is deferred to -[#20](https://github.com/eqrm/ct-cli/issues/20). Until then, assign to an -**existing** campus by hardcoding its numeric id, or apply the campuses first and -fill in the ids on a second pass. +Link a group to a campus **by key** — `campus: "mainz"` (or, when the campus key +is a loop variable, `campus`) — and the per-host resolver fills in the id (#20). +When the blueprint *creates* the campus in the same apply, its id is unknown at +eval time, so the resolver marks the link **pending** and writes the +freshly-created id at apply time (tier ordering creates the campus first). `ct +plan` renders it as `campusId = `. + +The same portability applies to the group type: `groupType: "ministry_team"` +resolves against the live catalog per host, no hardcoded `groupTypeId`. + +```ts +function kidsArea(ct: ConfigContext, campus: string): void { + const lead = `${campus}_kids_lead`; + // group type BY NAME, campus BY KEY — both resolved per host (#20). + ct.group({ key: lead, name: `${campus} · Kids Leitung`, groupType: "ministry_team", campus, parents: [] }); +} +``` + +The **numeric escape hatch** stays available: pass `campusId: ` +(CT stores it at `information.campusId`; `campusId: null` clears it) or +`groupTypeId: 2` to target one instance's id directly. `ct plan` diffs a campus +assign/move/clear as a normal field update — see +[`docs/group-field-decisions.md`](group-field-decisions.md). Declaring both the +logical and the numeric form for one field (`campus` + `campusId`) is a conflict +and throws at eval time. ## The loop-over-campuses pattern and `${campus}_`-prefixed keys diff --git a/docs/dynamic-groups.md b/docs/dynamic-groups.md index 1780582..01f1ea2 100644 --- a/docs/dynamic-groups.md +++ b/docs/dynamic-groups.md @@ -106,9 +106,13 @@ sibling of `query`, not an argument to `churchQuery(...)`. | `q.oneof(varName, values)` | `{ oneof: [{ var: varName }, values] }` | | `q.isnull(varName)` | `{ isnull: [{ var: varName }] }` | -`var` values are **raw ChurchTools ids** (e.g. `ctgroup.campusId`) — resolve -any key → id lookup at config-build time, before calling `q.eq`/`q.oneof`, -and pass the number. +`var` values may be **logical references** or **raw ids**. Prefer a reference so +the ruleset is portable across hosts (#20): `q.eq("ctgroup.campusId", +ref.campus("mainz"))` — the per-host resolver fills in that instance's campus id +at plan time (and, for a campus created in the same run, at apply time). `ref` is +re-exported from `src/config/context.js` alongside `q`/`churchQuery`. The numeric +escape hatch still works — pass a plain number to target one instance's id +directly. References resolve deep inside the ruleset, so any `var` value works. `churchQuery(filter, opts?)` wraps a JSONLogic filter tree in the same envelope shape ChurchTools itself returns: @@ -131,7 +135,7 @@ covers `primaryEntityAlias` / `responseFields` / `groupBy`, for a query keyed on something other than `person.id`. ```ts -import { q, churchQuery } from "../src/config/context.js"; +import { q, churchQuery, ref } from "../src/config/context.js"; const ruleset = { description: "Alle aktiven Personen in Mainz", @@ -139,7 +143,8 @@ const ruleset = { importance: 0, personIdFieldName: "person.id", process: {}, - query: churchQuery(q.and(q.eq("ctgroup.campusId", mainzCampusId), q.eq("person.isArchived", false))), + // campus BY NAME — resolved to the per-host id at plan time (numeric ids still work too). + query: churchQuery(q.and(q.eq("ctgroup.campusId", ref.campus("mainz")), q.eq("person.isArchived", false))), }; ``` diff --git a/docs/permissions.md b/docs/permissions.md index 1090bc7..90da60a 100644 --- a/docs/permissions.md +++ b/docs/permissions.md @@ -9,8 +9,8 @@ workflow used for structural resources (issue #13). ```ts export default (ct) => { ct.groupTypeRole({ - key: "leiter_tpl", // logical key (unique across the whole config) - id: 8, // the domainId — see "domainId semantics" below + key: "leiter_tpl", // logical key (unique across the whole config) + groupType: "ministry_team", // domain BY NAME — resolved to the domainId per host (#20) grants: [ "churchgroup:view group", // unscoped { right: "churchgroup:view group", scope: ["kids_area"] }, // scoped @@ -19,20 +19,27 @@ export default (ct) => { ct.groupRole({ key: "kids_lead_grant", - id: 2882, // the internal (group, role) domainId — see below + id: 2882, // the internal (group, role) domainId — see below (group_role has no ref yet) // "edit group memberships of group" is a scoped right, so it takes a `scope: [...]`. grants: [{ right: "churchgroup:edit group memberships of group", scope: ["kids_area"] }], }); }; ``` -Both take the same shape, `{ key, id, grants }`: +Both take `{ key, , grants }`, where `` is either a logical +reference or a numeric `id`: - **`key`** — the logical key, unique across the whole config (shared namespace with every other resource type). -- **`id`** — the explicit **domainId** of the permission domain object. This - tool does not look it up for you; you supply it directly (see - "domainId semantics" below). +- **domain** — the permission domain object. Declare it **by reference** (the + portable form, #20) or **by numeric `id`** (the escape hatch): + - `ct.groupTypeRole` — `groupType: ""` resolves against the live + group-type catalog per host, or `id: ` targets one directly. + - `ct.groupRole` — **`id: ` only for now.** The logical + `group: "", role: ""` form is accepted by the DSL but the + resolver rejects it at plan time (the (group, role) pairing id has no + confirmed API source — see "domainId semantics" and #25). Declaring both a + logical form and a numeric `id` is a conflict and throws. - **`grants`** — an array of `Grant`s, each either: - a bare string, `"module:right"` — an **unscoped** grant, or - an object `{ right: "module:right", scope: string[] }` — a **scoped** @@ -67,16 +74,25 @@ evaluation only checks a grant's *shape*: `module:right` string or The two DSL functions manage two different ChurchTools "domain types," and `id` means something different for each: -- **`group_type_role`** (`ct.groupTypeRole`) — `id` is the **group type's own - id** (the same id you'd pass as `groupTypeId` on `ct.group`). It scopes the - grant to "every role holder of this group type." -- **`group_role`** (`ct.groupRole`) — `id` is the **internal +- **`group_type_role`** (`ct.groupTypeRole`) — the domain is the **group type's + own id** (the same id you'd pass as `groupTypeId` on `ct.group`). It scopes the + grant to "every role holder of this group type." Declare it portably as + `groupType: ""` (resolved per host, #20) or directly as `id: `. +- **`group_role`** (`ct.groupRole`) — the domain is the **internal (group, role) pairing's own id** — a ChurchTools-internal id for one specific group's specific role, *not* the group's id and *not* the role's - id. There is no lookup helper for this in the CLI; find it via the - ChurchTools permission editor / an existing `GET /permissions/group_role` - response for a group+role you already have, and hardcode it in the config - like any other domainId. + id. **This is `id: ` only.** The logical `group` + `role` form is + reserved (and accepted by the DSL) but **not yet resolvable**: the pairing id + has no confirmed API source, so the resolver throws a clear "pass a numeric id + (see #25)" error at plan time. Find the id via the ChurchTools permission + editor / an existing `GET /permissions/group_role` response for a group+role + you already have, and hardcode it like any other domainId. + +Resolution runs in `buildPermissionPlan` (`src/permissions/plan.ts`): a numeric +`id` passes straight through; a `groupType` reference resolves against the live +catalog. After resolution, two declarations that resolve to the **same** +`(domainType, domainId)` are rejected (they would otherwise diff against each +other's grants forever) — even if one used a name and the other a raw id. ## Scope resolution diff --git a/docs/runbook-manual-surface.md b/docs/runbook-manual-surface.md index fd1e29d..1d9038c 100644 --- a/docs/runbook-manual-surface.md +++ b/docs/runbook-manual-surface.md @@ -33,11 +33,10 @@ this doc's structure. | Item | What it is | Tracking issue | Manual workaround today | | ------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| Group ↔ campus assignment (same-run) | Assigning a group to a campus **created in the same `ct apply`** — needs the new campus's id at eval time, which is unknowable without the logical-reference resolver. Assignment to an **existing** campus by numeric `campusId` **is managed now** (#21) | [#20](https://github.com/eqrm/ct-cli/issues/20) | Apply the campus first, look up its id (`ct get campuses`), then set `campusId: ` on the group and apply again; or assign by hand in the CT admin page. `plan` diffs the numeric `campusId` as a normal field update | | Group/group-type field decision table | Fields deliberately left unmanaged (decided out of scope): visibility, note, `autoAccept`/open-for-members, chat status, sort key. The triage **shipped** as a committed decision table ([`docs/group-field-decisions.md`](group-field-decisions.md)) | [#21](https://github.com/eqrm/ct-cli/issues/21) (decided) | Set by hand; these fields are intentionally not diffed — `ct` will neither preserve nor revert them. Promote one later only with its own registry entry + tests | -| Portable/logical references | Config still hardcodes numeric CT ids (`groupTypeId`, `groupStatusId`, `campusId`, permission `domainId`, dynamic-group ruleset `var` values like `q.eq("ctgroup.campusId", 4)`) instead of resolving keys/names per host | [#20](https://github.com/eqrm/ct-cli/issues/20) | Hand-resolve each id per target host (`ct get group-types`, `ct get campuses`, etc.) and hardcode it in config; a config authored against one instance will not plan correctly against another until this lands | +| Portable/logical references | **Shipped (#20).** Configs reference master data by name/key — `campus`/`groupType`/`status` on a group, `ref.campus(...)` in ruleset `var` values, `groupType: ""` for a `group_type_role` domain — and the per-host resolver maps each to that instance's id at plan time (managed resources ∪ live catalogs). A same-run campus resolves at apply time. Numeric ids still work as an escape hatch. **Residual gap:** the `group_role` domain by (group, role) reference is still gated — see the row below | [#20](https://github.com/eqrm/ct-cli/issues/20) (done) | None needed for the shipped surface. Write logical names; run `ct plan`. For the gated `group_role` case, use a numeric `id` (next row) | | Environments (dev → prod promotion) | Named `(host, token, state file)` profiles and a `--env` flag; today one config + one state file = one host | [#22](https://github.com/eqrm/ct-cli/issues/22) | Point `CT_HOST`/state file manually at each target and re-run; keep dev and prod state files apart yourself, and be careful — nothing stops you from applying a dev-shaped config against prod today | -| Permission `domainId` by reference | `ct.groupRole`/`ct.groupTypeRole` require the numeric `domainId` supplied by hand — for `group_role` this is CT's internal (group, role) _pairing_ id, with **no CLI lookup helper** | [#25](https://github.com/eqrm/ct-cli/issues/25) | Find the pairing id via the CT permission editor, or an existing `GET /permissions/group_role` response for a group+role you already have, and hardcode it ([`docs/permissions.md`](permissions.md) "domainId semantics") | +| Permission `group_role` domain by reference | `group_type_role` domains now resolve by name (`groupType: ""`, #20). But `group_role`'s domain is CT's internal (group, role) _pairing_ id, with **no confirmed API source** — the DSL accepts `group: "", role: ""` but the resolver rejects it at plan time with a "pass a numeric id" error | [#25](https://github.com/eqrm/ct-cli/issues/25) | Find the pairing id via the CT permission editor, or an existing `GET /permissions/group_role` response for a group+role you already have, and pass it as numeric `id` ([`docs/permissions.md`](permissions.md) "domainId semantics") | | ~~Grant adoption~~ **(shipped)** | ~~existing rights structures must be hand-transcribed~~ — **`ct adopt grants ` ships this** (#25): it reads the live rows, applies the planner's normalization, and prints a paste-ready `ct.groupRole` / `ct.groupTypeRole` block (baseline/inherited excluded, denies noted-and-preserved, scope dataIds mapped back to managed-group keys). See [`docs/permissions.md`](permissions.md) "Adopting existing grants" | [#25](https://github.com/eqrm/ct-cli/issues/25) (done) | No workaround needed — run `ct adopt grants group_role ` (or `group_type_role`), review the `WARNING`/`NOTE` comments, paste into config | | Permission catalog lifecycle | `catalog.json` is a one-off HAR-trace snapshot of a single CT version, with no staleness detection | [#25](https://github.com/eqrm/ct-cli/issues/25) | Manual regeneration procedure below (**Permission catalog lifecycle**) | | API re-audit for new CT releases | CT's OpenAPI spec is self-trimming (only shows endpoints your version has), so a new write endpoint (e.g. a group-status write) appears silently between CT upgrades | tracked by this issue ([#26](https://github.com/eqrm/ct-cli/issues/26)) | Procedure below (**Re-audit procedure for new CT releases**) | @@ -101,10 +100,11 @@ in where they'd otherwise be silently skipped: 1. `ct apply` the structural config (campuses, group types, age/target groups, groups, hierarchy, dynamic groups, permission grants). -2. **Group ↔ campus assignment** — declare `campusId: ` - on the group in config; `ct` diffs and applies it like any field (#21). - Only assignment to a campus created in the *same* apply is still manual - (#20) — apply the campus first, then set its numeric id. +2. **Group ↔ campus assignment** — declare `campus: ""` (or numeric + `campusId: `) on the group in config; `ct` diffs and applies it like any + field (#21). Same-run campuses now resolve automatically — the resolver marks + the link pending at plan time and fills in the freshly-created id at apply + time (#20), so no second pass is needed. 3. **Member statuses** — confirm the expected set exists via `ct get raw /group/memberstatus`; create any missing ones by hand in the CT admin UI. diff --git a/examples/campus-blueprint.config.ts b/examples/campus-blueprint.config.ts index 33d68d5..c0a8ca1 100644 --- a/examples/campus-blueprint.config.ts +++ b/examples/campus-blueprint.config.ts @@ -11,19 +11,22 @@ const CAMPUSES = ["mainz", "berlin"] as const; /** * One campus's Kids area: a lead group with three ministry teams under it, plus a dynamic "all members" group. * - * Campus assignment note (#21): a group is assigned to a campus with a numeric `campusId: `. - * These campuses are created in this same apply, so their ids are unknowable at eval time — linking a group - * to a *same-run* campus by key (`campus: "mainz"`) is the logical-reference resolver's job and lands with #20. - * To assign to an existing campus today, pass its numeric id, e.g. `campusId: 3`. + * Portable references (#20): the group type is named (`groupType: "ministry_team"`) rather than a + * hardcoded numeric id — the per-host resolver maps it to that instance's group-type id at plan time. + * Campus assignment (#21) uses the SAME logical form: `campus: campus` links each group to a campus + * created in this very apply. Its id is unknowable at eval time, so the resolver marks it pending and + * fills in the freshly-created id at apply time. The numeric escape hatch still works everywhere: + * pass `groupTypeId: 2` / `campusId: 3` to target an existing id directly. */ function kidsArea(ct: ConfigContext, campus: string): void { const lead = `${campus}_kids_lead`; - ct.group({ key: lead, name: `${campus} · Kids Leitung`, groupTypeId: 2, parents: [] }); + ct.group({ key: lead, name: `${campus} · Kids Leitung`, groupType: "ministry_team", campus, parents: [] }); for (const [suffix, label] of [["0_3", "0–3"], ["4_6", "4–6"], ["checkin", "Check-in"]] as const) { ct.group({ key: `${campus}_kids_${suffix}`, name: `${campus} · Kids ${label}`, - groupTypeId: 2, + groupType: "ministry_team", + campus, parents: [lead], // managed hierarchy: team sits under the campus lead group }); } @@ -31,7 +34,8 @@ function kidsArea(ct: ConfigContext, campus: string): void { ct.group({ key: `${campus}_kids_all`, name: `${campus} · Kids (alle)`, - groupTypeId: 2, + groupType: "ministry_team", + campus, parents: [lead], dynamic: { status: "manual", @@ -51,13 +55,14 @@ export default (ct: ConfigContext): void => { ct.campus({ key: campus, name: `Campus ${campus}`, shorty: campus.slice(0, 3).toUpperCase() }); kidsArea(ct, campus); } - // A permission grant (#13) on a shared group-type-role template — id is an illustrative placeholder. - // Both rights are scoped (they carry a `scopeField` in the catalog), so they must be declared - // as `{ right, scope: [...] }` — a bare string would grant them globally and is rejected. + // A permission grant (#13) on a shared group-type-role template. The domain is declared by name + // (`groupType: "ministry_team"`) — no hardcoded numeric domainId. Both rights are scoped (they + // carry a `scopeField` in the catalog), so they must be declared as `{ right, scope: [...] }` — + // a bare string would grant them globally and is rejected. const kidsLeads = CAMPUSES.map((c) => `${c}_kids_lead`); ct.groupTypeRole({ key: "kids_lead_tpl", - id: 2, + groupType: "ministry_team", grants: [ { right: "churchgroup:view group", scope: kidsLeads }, { right: "churchgroup:edit group memberships of group", scope: kidsLeads }, diff --git a/examples/dynamic-group.config.ts b/examples/dynamic-group.config.ts index 70eeeba..f0e1b3a 100644 --- a/examples/dynamic-group.config.ts +++ b/examples/dynamic-group.config.ts @@ -3,14 +3,14 @@ * built with the typed query DSL. See docs/dynamic-groups.md for the full * feature guide. * - * This is illustrative only — `mainzCampusId` below is a placeholder. In a - * real config, resolve name → id lookups (e.g. from `ct get campuses`) at - * config-build time before calling `q.eq`. + * Portable references (#20): the ruleset's `campusId` filter is written as a + * logical `ref.campus("mainz")` instead of a hardcoded numeric id — the per-host + * resolver fills in that instance's campus id at plan time. Here "mainz" is even + * created in the same run, so the resolver links to its freshly-created id at + * apply time. The numeric escape hatch still works: pass a plain number to `q.eq`. */ import type { ConfigContext } from "../src/config/context.js"; -import { q, churchQuery } from "../src/config/context.js"; - -const mainzCampusId = 0; // placeholder — replace with the real campus id +import { q, churchQuery, ref } from "../src/config/context.js"; export default (ct: ConfigContext): void => { ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); @@ -40,7 +40,7 @@ export default (ct: ConfigContext): void => { personIdFieldName: "person.id", process: {}, query: churchQuery( - q.and(q.eq("ctgroup.campusId", mainzCampusId), q.eq("person.isArchived", false)), + q.and(q.eq("ctgroup.campusId", ref.campus("mainz")), q.eq("person.isArchived", false)), ), }, }, diff --git a/examples/permissions.config.ts b/examples/permissions.config.ts index e49f686..fe51ba2 100644 --- a/examples/permissions.config.ts +++ b/examples/permissions.config.ts @@ -3,22 +3,24 @@ * grant and one scoped grant. See docs/permissions.md for the full feature * guide, and `ct get permissions-catalog` to discover right names. * - * This is illustrative only — the `id: 1` on `groupTypeRole` below is a - * placeholder domainId (the group type's own id; see docs/permissions.md - * "domainId semantics"). In a real config, use the actual group type id - * (e.g. from `ct get group-types`). + * Portable references (#20): the permission domain is declared by name + * (`groupType: "kids"`) instead of a hardcoded numeric domainId — the per-host + * resolver maps it to that instance's group-type id at plan time. The numeric + * escape hatch still works: pass `id: ` to target one directly (see + * docs/permissions.md "domainId semantics"). */ import type { ConfigContext } from "../src/config/context.js"; export default (ct: ConfigContext): void => { // The scoped grant below references this group by its logical key. Scope // keys must resolve to a group managed by this tool (declared or adopted), - // so ct plan can show what the grant's scope resolves to. - ct.group({ key: "kids_area", name: "Kids · Bereich", groupTypeId: 1 }); + // so ct plan can show what the grant's scope resolves to. The group's own + // type is named too — the resolver fills in the id per host. + ct.group({ key: "kids_area", name: "Kids · Bereich", groupType: "kids" }); ct.groupTypeRole({ key: "leiter_tpl", - id: 1, // placeholder — the real group type's id (e.g. from `ct get group-types`) + groupType: "kids", // logical domain — the resolver maps it to the group type's id per host grants: [ // Unscoped: applies everywhere this group type's role holds. authId 1101. "churchgroup:view", diff --git a/examples/portable.config.ts b/examples/portable.config.ts new file mode 100644 index 0000000..fa5093e --- /dev/null +++ b/examples/portable.config.ts @@ -0,0 +1,57 @@ +/** + * Portable config (#20): ZERO numeric ChurchTools ids. Every id-bearing field is + * a logical reference the per-host resolver fills in at plan time — so this exact + * file plans and applies against any instance without edits (ids differ per host). + * + * References resolve from, in order: (1) resources managed by this tool (declared + * here or already in state), (2) live master-data catalogs (group types, statuses, + * campuses, roles) matched by name. An unresolvable name fails the plan with a + * clear error naming the reference and where it was used — never a silent wrong id. + * + * The numeric escape hatch remains available everywhere (`groupTypeId: 2`, + * `campusId: 3`, `q.eq("ctgroup.campusId", 4)`, `id: `) for the rare case + * where you deliberately target one instance's id. + */ +import type { ConfigContext } from "../src/config/context.js"; +import { q, churchQuery, ref } from "../src/config/context.js"; + +export default (ct: ConfigContext): void => { + // A campus created in this same run. Groups below link to it by key (`campus: "mainz"`); + // its id is unknown until it is created, so the resolver marks those links pending and + // fills in the real id at apply time (tier ordering creates the campus first). + ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); + + // Group type + campus by name — no numeric ids. `groupType: "ministry_team"` resolves against + // the live /group/grouptypes catalog on whichever host you apply to. + ct.group({ key: "mainz_kids_lead", name: "Mainz · Kids Leitung", groupType: "ministry_team", campus: "mainz", parents: [] }); + ct.group({ key: "mainz_kids_team", name: "Mainz · Kids Team", groupType: "ministry_team", campus: "mainz", parents: ["mainz_kids_lead"] }); + + // A dynamic auto-group whose ruleset filters by campus — again by reference, not id. + ct.group({ + key: "mainz_kids_all", + name: "Mainz · Kids (alle)", + groupType: "ministry_team", + campus: "mainz", + parents: ["mainz_kids_lead"], + dynamic: { + status: "manual", + ruleset: { + description: "Alle aktiven Kids-Mitarbeiter Mainz", + importance: 0, + personIdFieldName: "person.id", + process: {}, + query: churchQuery( + q.and(q.eq("ctgroup.campusId", ref.campus("mainz")), q.eq("person.isArchived", false)), + ), + }, + }, + }); + + // A permission template whose domain is the group type BY NAME — the resolver maps it to the + // per-host group-type id. Scope keys reference the managed groups above. + ct.groupTypeRole({ + key: "kids_lead_tpl", + groupType: "ministry_team", + grants: [{ right: "churchgroup:view group", scope: ["mainz_kids_lead"] }], + }); +}; From 43bb2463c63e0b2b3dc9e4f87946e547c8fc1ebd Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 9 Jul 2026 09:13:23 +0200 Subject: [PATCH 4/5] fix(refs): re-resolve pending refs inside synthetic-field changes too (#20) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A same-run reference embedded in a dynamic ruleset's var value was applied via applySyntheticFields, bypassing body re-resolution — the pending sentinel would leak into the ruleset PUT. Re-resolve the whole change set up front so both the write body and synthetic-field writes see real ids. Lock it with a test. --- src/engine/execute.ts | 26 +++++++++++++++----------- tests/portable-refs.test.ts | 32 +++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 12 deletions(-) diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 59e5961..0c62e4c 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -58,11 +58,13 @@ async function applySyntheticFields( export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise { const { client, state, statePath } = deps; - // Re-resolve any pending logical reference (#20) against the current state before a body/snapshot - // is used. Tier ordering guarantees a referenced target (e.g. a same-run campus, tier 0) is already - // in state by the time its referencer (a group, tier 1) applies. No-op when nothing is pending. - const reresolve = (fields: Record): Record => - hasPendingRef(fields) ? (reresolvePendingValue(fields, state) as Record) : fields; + // Re-resolve every pending logical reference (#20) in an item's changes against the current state, + // up front — so both the write body AND synthetic-field writes (a dynamic ruleset's `var` value can + // reference a same-run resource) see real ids, never a pending sentinel. Tier ordering guarantees a + // referenced target (e.g. a same-run campus, tier 0) is already in state by the time its referencer + // (a group, tier 1) applies. No-op when nothing is pending. + const reresolveChanges = (changes: FieldChange[]): FieldChange[] => + changes.map((c) => (hasPendingRef(c.to) ? { ...c, to: reresolvePendingValue(c.to, state) } : c)); const now = deps.now ?? (() => new Date().toISOString()); const save = deps.save ?? saveState; const created: string[] = []; @@ -97,8 +99,10 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise("POST", spec.collectionPath, body); if (typeof res.id !== "number") { @@ -112,7 +116,7 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise !isSyntheticField(c.field)); + const snapshot = snapshotFromChanges(actualFields, changes); + const hasFieldChange = changes.some((c) => !isSyntheticField(c.field)); if (hasFieldChange) { // PATCH resources take only the changed fields (unchanged/drifted siblings are left alone); // PUT resources replace the whole object, so send actual ∪ changes to preserve those siblings. - const body = spec.updateMethod === "PATCH" ? reresolve(snapshotFromChanges({}, item.changes)) : snapshot; + const body = spec.updateMethod === "PATCH" ? snapshotFromChanges({}, changes) : snapshot; const path = spec.itemPath(id); assertNotPeople(path); await client.request(spec.updateMethod, path, body); @@ -137,7 +141,7 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise { // State records the resolved id too (no pending marker leaks into state). expect(state.resources.kids?.fields).toMatchObject({ campusId: 42 }); }); + + it("resolves a same-run ref embedded in a dynamic ruleset before the ruleset PUT", async () => { + // The dynamic group is already managed (a fresh dynamic group is a two-apply flow); the campus it + // filters on is added to the config now, so its ruleset ref is same-run pending until apply time. + const { resources } = await evaluateConfig((ct) => { + ct.campus({ key: "mainz", name: "Mainz", shorty: "MZ" }); + ct.group({ + key: "all_mainz", + name: "All", + groupTypeId: 1, + dynamic: { + status: "manual", + ruleset: { query: churchQuery(q.eq("ctgroup.campusId", ref.campus("mainz"))) }, + }, + }); + }); + const state = emptyState("h"); + state.resources.all_mainz = { + type: "group", id: 100, key: "all_mainz", fields: { name: "All", groupTypeId: 1 }, adoptedAt: "t", updatedAt: "t", + }; + // /groups/100 fetches clean; its ruleset 404s (not yet a dynamic group) → the manual ruleset is a change. + const host = fakeHost({ "/groups/100": { name: "All", groupTypeId: 1 } }, { "POST /campuses": 42 }); + const { plan } = await buildPlan(host, state, resources); + await executePlan(plan, { client: host, state, statePath: "s.json", save: noSave, now: () => "t" }); + + const rulesetPut = host.calls.find((c) => c.path === "/dynamicgroups/100/ruleset" && c.method === "PUT")!; + const body = rulesetPut.body as { dynamicGroupRuleSet: { query: { params: { filter: { "==": unknown[] } } } } }; + // The campus ref, pending at plan time, is the freshly-created id (42) in the PUT — not a sentinel. + expect(body.dynamicGroupRuleSet.query.params.filter["=="][1]).toBe(42); + }); }); describe("permission domainId resolution", () => { From 30b59de753ba646f93ef2bb05b7588cd29312286 Mon Sep 17 00:00:00 2001 From: Felix Kotschenreuther Date: Thu, 9 Jul 2026 10:08:27 +0200 Subject: [PATCH 5/5] fix(refs): dependency-order same-tier pending ref targets before their referencers A pending ref names a same-run resource, but tier ordering alone doesn't put the target first when referencer and target share a tier (a group's ruleset ref.group()-ing another group applied in declaration order and could never converge). buildPlan now injects a dependsOn edge from the referencer to each pending target so orderKeys sequences them; the pending-unresolved apply error no longer misdirects to tier ordering. Flagged by the PR #46 review with an empirical repro; test locks the create-before-ruleset-PUT order and the resolved id. --- src/engine/build.ts | 15 ++++++++++++++- src/resolve/refs.ts | 25 +++++++++++++++++++++++++ src/resolve/resolver.ts | 3 ++- tests/portable-refs.test.ts | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 2 deletions(-) diff --git a/src/engine/build.ts b/src/engine/build.ts index f28a1f4..fa59789 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -13,6 +13,7 @@ import { RESOURCES } from "../resources/registry.js"; import { computePlan } from "./plan.js"; import { foldSynthetic } from "./synthetic.js"; import { Resolver } from "../resolve/resolver.js"; +import { collectPendingRefKeys } from "../resolve/refs.js"; import { mapConcurrent } from "../util/concurrency.js"; import { warn } from "../ui.js"; @@ -122,6 +123,18 @@ export async function buildPlan( }), ); - const plan = computePlan(resolved, state, actual, { unresolved, fetchFailed }); + // A pending ref names a resource created in this same run, but tier ordering alone doesn't put + // the target first when both share a tier (e.g. a group's ruleset ref.group()-ing another group: + // declaration order would apply the referencer first and the pending id could never resolve). + // Inject the dependency edge so orderKeys sequences the target before the referencer. + const desiredKeys = new Set(resolved.map((d) => d.key)); + const ordered = resolved.map((d) => { + const targets = [ + ...new Set(collectPendingRefKeys(d.fields).filter((k) => k !== d.key && desiredKeys.has(k))), + ].filter((k) => !d.dependsOn.includes(k)); + return targets.length === 0 ? d : { ...d, dependsOn: [...d.dependsOn, ...targets] }; + }); + + const plan = computePlan(ordered, state, actual, { unresolved, fetchFailed }); return { plan, actual, fetchErrors }; } diff --git a/src/resolve/refs.ts b/src/resolve/refs.ts index a5bae91..3ccecb2 100644 --- a/src/resolve/refs.ts +++ b/src/resolve/refs.ts @@ -137,6 +137,31 @@ export function collectRefs(value: unknown): Ref[] { return out; } +/** + * Collect the managed logical keys named by every {@link PendingRef} in a value. Pending markers + * always point at a same-run declared resource, so these keys are exactly the apply-order + * dependencies the referencing resource needs (group-role refs are gated and never go pending). + */ +export function collectPendingRefKeys(value: unknown): string[] { + const out: string[] = []; + const walk = (v: unknown): void => { + if (isPendingRef(v)) { + const r = v.__pendingRef; + if (r.kind !== "group-role") out.push(r.key); + return; + } + if (Array.isArray(v)) { + v.forEach(walk); + return; + } + if (v !== null && typeof v === "object") { + for (const x of Object.values(v as Record)) walk(x); + } + }; + walk(value); + return out; +} + /** True when a value contains at least one {@link PendingRef} marker (short-circuit for apply-time rewrite). */ export function hasPendingRef(value: unknown): boolean { if (isPendingRef(value)) return true; diff --git a/src/resolve/resolver.ts b/src/resolve/resolver.ts index faad57a..d501413 100644 --- a/src/resolve/resolver.ts +++ b/src/resolve/resolver.ts @@ -226,7 +226,8 @@ function pendingIdFromState(r: Ref, state: State): number { if (!managed) { throw new Error( `Pending reference ${refLabel(r)} did not resolve after apply — "${r.key}" is not in state. ` + - `Its tier should have applied first.`, + `buildPlan orders the target before its referencer (injected dependency edge), so this ` + + `usually means the target's create failed earlier in this run.`, ); } return managed.id; diff --git a/tests/portable-refs.test.ts b/tests/portable-refs.test.ts index df73648..8b01fcb 100644 --- a/tests/portable-refs.test.ts +++ b/tests/portable-refs.test.ts @@ -116,6 +116,38 @@ describe("apply-time pending re-resolution (same-run campus + group)", () => { // The campus ref, pending at plan time, is the freshly-created id (42) in the PUT — not a sentinel. expect(body.dynamicGroupRuleSet.query.params.filter["=="][1]).toBe(42); }); + + it("orders a same-tier pending ref target before its referencer (group → ref.group)", async () => { + // PR #46 review finding: both groups are tier 1, and the referencer is declared FIRST. + // Declaration order alone would apply "all_kids" before "b_target" and the pending id could + // never resolve — the injected dependency edge must put the target first. + const { resources } = await evaluateConfig((ct) => { + ct.group({ + key: "all_kids", + name: "All Kids", + groupTypeId: 1, + dynamic: { + status: "manual", + ruleset: { query: churchQuery(q.eq("ctgroup.parentId", ref.group("b_target"))) }, + }, + }); + ct.group({ key: "b_target", name: "Target", groupTypeId: 1 }); + }); + const state = emptyState("h"); + state.resources.all_kids = { + type: "group", id: 100, key: "all_kids", fields: { name: "All Kids", groupTypeId: 1 }, adoptedAt: "t", updatedAt: "t", + }; + const host = fakeHost({ "/groups/100": { name: "All Kids", groupTypeId: 1 } }, { "POST /groups": 55 }); + const { plan } = await buildPlan(host, state, resources); + await executePlan(plan, { client: host, state, statePath: "s.json", save: noSave, now: () => "t" }); + + const createIdx = host.calls.findIndex((c) => c.method === "POST" && c.path === "/groups"); + const putIdx = host.calls.findIndex((c) => c.method === "PUT" && c.path === "/dynamicgroups/100/ruleset"); + expect(createIdx).toBeGreaterThanOrEqual(0); + expect(putIdx).toBeGreaterThan(createIdx); // target created before the referencing ruleset writes + const body = host.calls[putIdx]!.body as { dynamicGroupRuleSet: { query: { params: { filter: { "==": unknown[] } } } } }; + expect(body.dynamicGroupRuleSet.query.params.filter["=="][1]).toBe(55); // the fresh id, not a sentinel + }); }); describe("permission domainId resolution", () => {