Repository navigation
feat(plan): group hierarchy as an opt-in managed parents set-field - #12
Merged
Merged
Conversation
Groups already plan (registry campus/group/group-type); this adds group
hierarchy to the plan.
- DSL: group({ parents: [keys] }) (+ parent sugar). Opt-in — omit to leave a
group's hierarchy unmanaged; explicit [] means "no managed parents".
Multi-parent (CT hierarchy is a DAG). Parents contribute dependency edges
(parents-before-children ordering; cycles are rejected).
- engine/hierarchy.ts: parse /groups/hierarchies into id→parentIds; resolve a
group's actual managed parents (parent ids ∩ managed → keys, sorted);
applyHierarchy folds parents into desired.fields + actual as a set-field so
computePlan diffs it generically. Edges to UNMANAGED groups stay invisible
(managed-guard).
- plan: one bulk /groups/hierarchies fetch when groups are managed.
- 7 new tests (hierarchy parse/resolve/apply, DSL parents opt-in/dedup) = 96.
Live-verified: config matches ⇒ no changes; drop edge ⇒ ~ parents [x]->[];
add edge ⇒ ~ parents []->[x]; mutual parents ⇒ cycle rejected.
Refs #5
Resolve correctness/altitude findings from the code review of the opt-in `parents` hierarchy feature: - Decouple `parent` from managed hierarchy. In `main` `parent` was an ordering hint only; the PR silently turned it into a diffed field, so a group with `parent: <campus>` (or any non-group/unmanaged key) produced a permanent, non-convergent `parents` diff. `parent` is now purely a dependency edge again; hierarchy is managed solely via `parents`. - Validate that every `parents` key references a group declared in the same config (reject typos / non-group / unmanaged parents up front instead of emitting an unconvergeable plan). Fixes the broken README example, which referenced an undeclared `mainz_area`. - On a `/groups/hierarchies` fetch failure, leave `parents` undiffed rather than folding an empty map — the old path fabricated spurious "add all parents" changes while claiming the diff was merely "missing". - `applyHierarchy` now only injects `actual.parents` for groups that opted in, matching the desired-side guard (no phantom pseudo-field on pristine records) and honouring its own docstring. - Validate `parent`/`parents` input types (reject a non-array `parents`, a string spread into per-character keys, etc.); treat a falsy `parent` as absent. - Document that `fields.parents` is a hierarchy pseudo-field (logical keys) that apply must route to the per-edge endpoint, and that hierarchy edits surface as updates, not drift (no adopt baseline). Tests: 101 (+5). Typecheck, eslint, prettier clean. Claude-Session: https://claude.ai/code/session_01XmRK4CSYw1jZUkeBFM3abe
4 tasks
2000game
added a commit
to bwl21/ct-cli
that referenced
this pull request
Aug 26, 2026
…aborts Review findings on eqrm#154: - synthetic: a state-bound member-field id missing from the live response made the WHOLE group undiffable (build.ts turns rows: undefined into fetchFailed), silently freezing name/parents/dynamic and every other member field. Scope it to the affected pseudo-field instead — dropped from the desired side, so no replacement POST — and name the recovery (`ct destroy --member-field <identity>`) in the error. - adopt-group: an id-less row or two rows slugging to the same local key threw, and saveState only runs after the whole --children-of loop, so one bad group discarded every group already adopted. Warn and skip the group's member fields, matching the read-failure path. - member-fields: guard the defaultValue↔option-id match on both sides being non-null; "undefined" === "undefined" reported the field converged forever and never wrote the declared default. - member-fields: merge the { type, field } wrapper over the nested definition instead of keeping only `type`, so a wrapper-held id survives, and gate the unwrap on the nested object looking like a definition. - resolver: a stale state binding filtered by id alone, then errored "group eqrm#12 has no member field \"wahl\" (available: \"Wahl\")" — name the stale binding and how to clear it instead. - adopt-group: don't write `memberFields: {}`; destroy deletes the key when the map empties, so an empty map churned the state file. Claude-Session: https://claude.ai/code/session_016JBGhwKsanNFAK9cs7N9Uv
2000game
added a commit
that referenced
this pull request
Aug 26, 2026
* fix(member-fields): normalize live reconciliation * fix(adopt): persist member field identities * fix(member-fields): normalize live API shapes * fix(member-fields): scope stale-binding degradation, stop bulk-adopt aborts Review findings on #154: - synthetic: a state-bound member-field id missing from the live response made the WHOLE group undiffable (build.ts turns rows: undefined into fetchFailed), silently freezing name/parents/dynamic and every other member field. Scope it to the affected pseudo-field instead — dropped from the desired side, so no replacement POST — and name the recovery (`ct destroy --member-field <identity>`) in the error. - adopt-group: an id-less row or two rows slugging to the same local key threw, and saveState only runs after the whole --children-of loop, so one bad group discarded every group already adopted. Warn and skip the group's member fields, matching the read-failure path. - member-fields: guard the defaultValue↔option-id match on both sides being non-null; "undefined" === "undefined" reported the field converged forever and never wrote the declared default. - member-fields: merge the { type, field } wrapper over the nested definition instead of keeping only `type`, so a wrapper-held id survives, and gate the unwrap on the nested object looking like a definition. - resolver: a stale state binding filtered by id alone, then errored "group #12 has no member field \"wahl\" (available: \"Wahl\")" — name the stale binding and how to clear it instead. - adopt-group: don't write `memberFields: {}`; destroy deletes the key when the map empties, so an empty map churned the state file. Claude-Session: https://claude.ai/code/session_016JBGhwKsanNFAK9cs7N9Uv * docs(member-fields): re-sign handbook for the review fixes Group member fields: adoption now warns and skips on rows it cannot capture cleanly (no id, colliding local keys) instead of aborting the run; a stale state binding degrades to that one field rather than the whole group, and names `ct destroy --member-field` as the way out; the `{ type, field }` wrapper keeps whatever only the wrapper carries. dynamic-groups.md and permissions.md are re-signed unchanged — the synthetic and resolver edits are confined to the member-field paths those pages do not document. Claude-Session: https://claude.ai/code/session_016JBGhwKsanNFAK9cs7N9Uv --------- Co-authored-by: Felix Kotschenreuther <felix.kotschenreuther@gmail.com>
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.
Broadens
ct planto the real scaffold: groups + hierarchy. (Groups already planned via the registry; this adds hierarchy.) Read-only — noapplyyet.What & why
ChurchTools group hierarchy is a multi-parent DAG (
GET /groups/hierarchies→ per group, a list of parent ids). This models it as an opt-inparentsset-field on a group:parents→ hierarchy unmanaged;parents: []→ "no managed parents".Design note
Modeled as a set-field on the group (not a separate resource) so
computePlanstays generic and hierarchy shows in the group's diff. The per-edge write path (PUT/DELETE /groups/{id}/parents/{parentId}) is deferred toapply(Phase 4).Implementation
engine/hierarchy.ts:parentIdsByGroupId,managedParentKeys,applyHierarchy(folds parents into desired + actual).plan: one bulk/groups/hierarchiesfetch when groups are managed.group({ parents })+parentsugar, de-duped.Tests — 96 (+7)
Hierarchy parse/resolve/apply (incl. unmanaged-parent filtering), DSL parents opt-in/dedup.
Live-verified ✅
Real groups 8 (parent) + 1311 (child): config matches ⇒ No changes; drop edge ⇒
~ group.staerker parents: ["team_kidsdienst"] -> []; add edge ⇒~ parents: [] -> ["staerker"]; mutual parents ⇒ cycle rejected.Refs #5