Repository navigation
fix(dynamic): key portable ruleset roles by (group-type, role-name), not role-def (#76) - #87
Merged
Merged
Conversation
…not role-def (#76) PR #86 rewrote a captured `dynamic: true` ruleset's ChurchQuery `role.id` operands into `{ __ctRef }` `role-def` markers, resolving them via `/group/roles` by `slug(name)` alone. That is broken: a ruleset's `role.id` is a **groupTypeRoleId** (a role scoped to a group TYPE), and role names are NOT globally unique. Live prod `/api/group/roles` (46 roles, 2026-07-11) has 3 "Leiter", 6 "Organisator", 6 "Mitglied" — each on a different group type — so the resolver throws "ambiguous" and the marker is unresolvable. The (groupTypeId, name) pair IS unique (0 collisions across all 46 roles). This introduces a new compound ref kind `group-type-role` (`{ __ctRef, kind: "group-type-role", groupType, role }`), paralleling `group-role`, and: - refs.ts: new `GroupTypeRoleRef` + `ref.groupTypeRole`; refKey/refLabel and the pending-collection guards handle it. - resolver.ts: resolves the group-type key to this host's id (managed state ∪ `/group/grouptypes`), then picks the one `/group/roles` row matching that groupTypeId AND `slug(name)`; 0 / >1 is a hard error listing candidates. - query-refs.ts: removes the wrong `role.id → role-def` entry from `VAR_REF_KINDS`; portablizes `role.id` via a supplied role catalog (id → {groupTypeId, name}) + group-type id→key map, and also rewrites the out-of-query `process.*.handleMembership.groupTypeRoleId` field. Stays pure. - reverse.ts: adds `roleGroupTypeCatalog()` (id → {groupTypeId, name}). - adopt-group.ts: `--portable-rulesets` fetches the role catalog + group-type id→key map and passes them through. Byte-faithful round-trip covered with the real ct-structure fixture skidscheckinopsmz.json. Live re-localization of the ct-structure rulesets is handled separately by a human.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug in #86
PR #86 (portable rulesets, #76) rewrote a captured
dynamic: trueruleset's ChurchQueryrole.idoperands into logical{ __ctRef }role-defmarkers, resolved via/group/rolesbyslug(name)alone.That decision is broken. A ruleset's
role.idis a groupTypeRoleId — a role scoped to a group type — and role NAMES are not globally unique.Live evidence (prod
/api/group/roles, 46 roles, 2026-07-11)role-defresolver (src/resolve/resolver.ts) throws "ambiguous" when a slug matches multiple catalog rows, so these markers are unresolvable./group/rolesrow carries agroupTypeId, and (groupTypeId, name) is unique — 0 collisions across all 46 roles.The fix
A new compound ref kind
group-type-role, paralleling the existinggroup-role:{ "__ctRef": true, "kind": "group-type-role", "groupType": "<group-type-key>", "role": "<role-name>" }Chosen over reusing
role-def(bare name — the ambiguous currency this replaces) orgroup-role(a group-instance permission-pairing domainId via/groups/{id}/roles, a different id). The compound (group-type, role-name) pair is the minimal self-contained key that resolves uniquely per environment. Justified in a code comment onGroupTypeRoleRef.GroupTypeRoleRef,ref.groupTypeRole(groupType, role);refKey/refLabeland the pending-collection guards handle the new kind./group/grouptypescatalog), then picks the one/group/rolesrow whosegroupTypeIdmatches ANDslug(name) === slug(role). 0 or >1 is a hard error listing candidates (mirrors the existing ambiguous/not-found style). Reuses the cached catalog fetch.role.id → role-defentry fromVAR_REF_KINDS; portablizesrole.idvia a caller-supplied role catalog (id → {groupTypeId, name}) + group-type id→key map, emitting the compound marker (unmanaged group type → numeric + warning, the escape hatch). Also rewrites the out-of-queryprocess.*.handleMembership.groupTypeRoleIdfield via the same mechanism. Stays pure (no network).roleGroupTypeCatalog()returning id → {groupTypeId, name}.--portable-rulesetsnow fetches the role catalog + group-type id→key map and passes them intoportablizeRuleset.Tests
tests/fixtures/dynamic/portablize-skidscheckinopsmz.json(copied from ct-structure): ctgroup.id["112","8","1246"], role.id["84","85","17","16"],process...groupTypeRoleId: 66.deepMapRefsback with the same maps → equals the normalized original (including the process field).handleMembership.groupTypeRoleIdtest, resolver unit tests (incl. not-found / ambiguous).lint,typecheck,test(560 passed | 5 skipped) all green.Out of scope
Live re-localization of the actual ct-structure
rulesets/*.jsonfiles is handled separately by a human — this PR only fixes the portablize/resolve mechanism.