Repository navigation
fix: adoption & portability — #114, #115, #118, #119, #122, #123, #125, #127 - #129
Merged
Merged
Conversation
…#118, #119, #122, #123, #125, #127) The "Adoption & portability" milestone: adopting existing CT structure, un-adopting, and making declarations portable between hosts that materialize inheritance differently. #119 + #114 — reconciliation decided whether a declared grant was SATISFIED from how the live row was RECORDED, and provenance is not stable across two hosts of one instance even when the effective permissions are identical. The same right is system-authored (`modifiedPid === -1`) on one host and user-authored on the other; the same 15 group-member rights read `isInherited: true` on one host and `false` on the other (measured: 18 of 63 `group_role` domains carried inherited rows on one host, zero anywhere on the other). Omitting the right revoked it there; declaring it planned `+1 grant` here — so the domain had to be left undeclared, though nothing about it is genuinely undeclarable. Satisfaction is now judged against the EFFECTIVE set (`normalizeEffective`: direct ∪ inherited ∪ system baseline) while revocation stays judged against the OWNED set, so ct still never authors or revokes a row it did not write — it only stops proposing to re-author what the platform already grants. `ct adopt grants` emits the effective set to match, and names in a footer how many of the emitted grants are inherited or baseline on this host. #115 — `ct adopt grants` emitted `run `ct adopt department -1`` for CT's "alle" sentinel. `-1` is not an id: it means every value of the dimension on whatever host reads it, so it is already portable and there is no resource to adopt. It now gets a one-line explanation instead of a hint that sends you looking for something that cannot exist. In a real adoption run this fires on most of the broadly-scoped grants. #127 — `person.id` was the one entity var the ruleset portability audit never mentioned, so a raw person id frozen into a cross-host ruleset could only be found by reading the captured JSON by hand. It is reported now, worded as unfixable rather than pending: ct does not manage people, so the remedy is a decision, not a command. #125 — a ruleset's `role.id` portablized to a (group-type, role NAME) pair, and a duplicate role name on one type was unfixable in config: renaming edits CT master data (here, CT's own stock `leader`), and a numeric id differs per host with no env to branch on. Capture now prefers a managed `role-def` key resolved from state — the same "adopt the target to portablize it" move groups and campuses already use — falling back to the pair for an unmanaged role. The ambiguity error names the remedy that actually works. #123 — re-adopting an already-managed resource silently re-keyed it, which the "snapshot was refreshed" message did not mention. That happens on the documented ruleset-refresh workflow, which passes a LIST of ids — the one mode where `-k` is rejected. The config then matched nothing in state and the next plan read as "one to create, one to destroy" for an untouched resource. An adopted key is now kept (and the derived one reported) unless `--rekey` is passed; the ruleset filename follows the kept key, so a refresh overwrites the file the config points at instead of writing a second one. #122 — `ct state rm <type> <key>` un-adopts: removes the entry, makes no HTTP call, leaves the resource in ChurchTools. Backing out an adoption previously meant hand-editing the state file, and `ct destroy` is the opposite of what un-adopting means. Refuses a key the config still declares (that would plan a CREATE for a live resource) unless `--force`. #118 — campus was the only resource deriving its key from `shorty`, producing keys like `of`, `wu` and `swa` for Dietzenbach, Würzburg and Idstein. `shorty` is a display abbreviation an admin edits to make a UI column fit, so it is also less stable than the name. It now derives from `name` like everything else, falling back to `shorty` only when `name` is empty; `shorty` is still written as a managed field. BREAKING CHANGE: campus logical keys now derive from `name`, not `shorty`. A campus adopted without an explicit `-k` has a key that no longer matches what adoption would derive; re-adopt with `-k <existing-key>` to keep it, or `--rekey` to move to the name-derived one. BREAKING CHANGE: `ct adopt grants` now emits inherited and system-baseline grants, and reconciliation treats them as satisfying a declaration. A config that relied on an inherited right being revoked must now declare that revoke explicitly. Claude-Session: https://claude.ai/code/session_01V24GdD2F6LTGTF5xWoGq4i
…eview) Adoption emits the EFFECTIVE grant set since #114/#119, but the bulk gate in `ct adopt grants --group/--all-declarable` still judged the OWNED set, so gate and emitter disagreed in both directions: - a domain carrying its rights only as inherited rows counted as "no authored grants" and was skipped, leaving them undeclared for the other host's plan to put in toDelete and revoke — the exact regression #119 exists to fix, left in place on the documented way to adopt at scale; - an inherited grant on a dimension with no logical form was invisible to the declarability gate, so the domain passed --all-declarable and was emitted with a host-specific numeric dataId — the cross-environment misgrant that gate's own comment says it exists to prevent. `declarability` takes a `scope` option rather than switching wholesale: `ct coverage`'s headline number is authored grants, and inherited rows are somebody else's authorship. Default stays "authored", so coverage is untouched. Also in this review pass: - query-refs: a managed `role-def` now wins over the (group-type, role) pair only when that pair is genuinely ambiguous. `role-def` is the weaker ref off this host — the resolver falls back to a `/group/roles` lookup keyed on slug(name) alone, so on a host that never adopted the shared key it can resolve silently to a role on a different group type. The pair cannot fail that way. The #125 tests asserted the preference with a single-row catalog, i.e. never modelled the ambiguity they described; they now model both. - `ct state rm`: the still-declared guard also consults permission declarations. A key named only by a `groupRole` domain or a group-dimension scope passed a resources-only guard and then hard-errored on the next `ct plan` — after the state file had been written.
dynamic-groups: the managed `role-def` form is emitted only when the (group-type, role) pair actually collides — off-host it is the weaker reference of the two, since it resolves by bare name. permissions: bulk `ct adopt grants` judges declarability over the effective rows, the same set it emits. README: `state rm`'s refusal covers permission declarations, not only resources.
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.
Closes #114, closes #115, closes #118, closes #119, closes #122, closes #123, closes #125, closes #127.
The whole Adoption & portability milestone. Stacked conceptually on #128 (Apply correctness), but independent — this branches from
mainand the two do not touch the same lines.#119 + #114 — reconcile on effective grants, not on row provenance
The root cause both issues name: ct decided what it owns from how a row was recorded, and that differs per host for identical effective permissions.
meta.modifiedPid: -1(system)modifiedPid: 1(a person)isInherited: trueisInherited: falseNeither is hand-made — the second host is a copy, and the copy stamped a person id onto rows that are system rows upstream. Measured: 18 of 63
group_roledomains carried inherited rows on one host, zero anywhere on the other.Implemented as option 1 from #119, which subsumes #114:
normalizeEffective= direct ∪ inherited ∪ system baseline. A right the host already grants by any route needs no PUT.normalizeActual, unchanged. ct never authors and never revokes a baseline or inherited row, so "never fight the platform" holds.ct adopt grantsemits the effective set, which is the other half: without it the revoke in Adopted grants are not portable between hosts that materialize inheritance differently #119 survives, because a right dropped on adoption is undeclared on the host that materializes it directly and lands intoDelete. A footer names how many of the emitted grants are inherited/baseline here.A test drives the actual #119 scenario: adopt from the inherited host, then plan the emitted config against both — clean no-op on each.
#115 — the
-1"alle" sentinelct adopt department -1names a resource that cannot exist, and-1is not host-specific — it means every value of the dimension on whatever host reads it. Both the group-dimension and catalog-dimension emitters had this; both now emit a one-line explanation and no hint. Fires on most broadly-scoped grants in a real adoption run.#127 —
person.idin the portability auditAdded to the audit, worded as unfixable rather than pending: ct correctly does not manage people, so the remedy is a decision ("remove the clause or accept the divergence"), not a command. Covers the negated exclusion form too, which is the dangerous one —
person.id 1exists on every instance and is almost always an administrator.#125 — ruleset role refs
Capture now prefers
{ kind: "role-def", key }resolved from managed state when the role is adopted, falling back to the(groupType, role)pair otherwise. That makes the ambiguous case fixable in config by adopting the role under a shared key on both hosts — the same move that already works for groups and campuses.This is what #86 got wrong and #76 reverted: the mapping was right, the name-keyed catalog it sourced from was not. State gives a per-host id under a shared key, which is unambiguous by construction.
The ambiguity error no longer offers two remedies that cannot work on a multi-host config.
#123 — re-adopt keeps the adopted key
An already-managed resource keeps its key unless
--rekeyis passed, and the derived key is reported rather than silently applied. Since the ruleset path is built from the key, this also makes a refresh overwrite the file the config points at instead of writing a second one.--rekeyadded to bothct adopt <type> <id>andct adopt group.#122 —
ct state rmRemoves the entry, makes no HTTP call, leaves the resource in ChurchTools. Refuses a key the config still declares — that would make the next plan propose creating a resource that already exists — unless
--force. A config that cannot be read downgrades the guard to a warning, since backing out an adoption is exactly what you do mid-edit. The test assertsauthedSessionis never called.#118 — campus keys from
namederiveKeynow matches every other resource.shortyis still written as a managed field.Breaking changes
name. A campus adopted without-kno longer matches what adoption derives — re-adopt with-k <existing-key>to keep it (the Re-adopting an already-managed resource silently changes its logical key #123 fix means a plain re-adopt now keeps it anyway), or--rekeyto move.ct adopt grantsemits inherited and baseline grants, and reconciliation treats them as satisfying a declaration. A config relying on an inherited right being revoked must now declare that revoke explicitly.npm run typecheck,npx eslint src tests,npx prettier --check— cleannpx vitest run— 776 passed, 5 skipped (was 748; +28 new)npm run build+ smoke-testedct state rmagainst the built binarydocs-staleness.mjs— all 5 pages current;permissions.mdgained a "Provenance and portability" section,dynamic-groups.mdgained the Ruleset role refs are (group-type, name) pairs, so a duplicate role name is unfixable in config #125/Ruleset portability warnings are silent about raw person.id literals #127/Re-adopting an already-managed resource silently changes its logical key #123 sections, README documentsstate rmhttps://claude.ai/code/session_01V24GdD2F6LTGTF5xWoGq4i