diff --git a/docs/dynamic-groups.md b/docs/dynamic-groups.md index 01f1ea2..14a63fc 100644 --- a/docs/dynamic-groups.md +++ b/docs/dynamic-groups.md @@ -172,6 +172,15 @@ never applied to ruleset-level string fields like `description`/`shorty`, so a numeric-looking label (e.g. a `description` of `"2024"`) is never silently retyped to a number and corrupted on write-back. +**Pinned assumption (#36):** the no-op-plan property above relies on every +RuleSet-level field OTHER than `query` (`description`, `shorty`, `importance`, +`personIdFieldName`, `process`) round-tripping through ChurchTools +byte-for-byte on `PUT`/`GET` — `normalizeRuleset` does not canonicalize them. +This is pinned by a live-gated test in `tests/dynamic.integration.test.ts` +(see `tests/fixtures/dynamic/README.md` for how to run it and what to do if +it fails); it is skipped by default and requires an explicit opt-in against a +dev instance. + ## Managed guard: undeclared groups stay invisible Dynamic-group state is only ever fetched for a group that is **both**: (a) diff --git a/tests/dynamic.integration.test.ts b/tests/dynamic.integration.test.ts index c14da88..daf4bef 100644 --- a/tests/dynamic.integration.test.ts +++ b/tests/dynamic.integration.test.ts @@ -1,9 +1,16 @@ import { describe, it, expect } from "vitest"; import { readFileSync } from "node:fs"; +import { isDeepStrictEqual } from "node:util"; import { normalizeRuleset } from "../src/engine/dynamic.js"; +import { diffFields } from "../src/engine/plan.js"; +import { syntheticField } from "../src/engine/synthetic.js"; import { authedSession } from "../src/api/session.js"; +import type { State } from "../src/state/state.js"; +import type { DesiredResource } from "../src/engine/types.js"; +import type { DynamicStatus } from "../src/engine/types.js"; const live = process.env.CT_LIVE === "1"; +const liveWrite = process.env.CT_LIVE_WRITE === "1"; const GID = Number(process.env.CT_DYNAMIC_FIXTURE_GID ?? "0"); // the group id from Task 1 describe.runIf(live)("dynamic round-trip (live)", () => { @@ -24,3 +31,122 @@ describe.runIf(live)("dynamic round-trip (live)", () => { expect(normalizeRuleset(nowRuleset)).toEqual(normalizeRuleset(fixture)); }); }); + +/** + * #36 pin: does a USER-AUTHORED ruleset — one this test writes itself, never copied from a prior + * GET — round-trip byte-for-byte (after normalization) through a PUT/GET cycle? + * + * The two tests above write back CT's OWN GET output, so they are drift-free by construction: if + * CT silently rewrote/normalized a RuleSet-level field (`description`, `shorty`, `importance`, + * `personIdFieldName`, `process` — see docs/dynamic-groups.md "Drift, normalization, and no-op + * re-applies") on PUT, those tests could never observe it, because the value they wrote back + * already matched. This test authors fresh field values instead, so it CAN observe such rewrites. + * If it fails, the failure message names exactly which RuleSet-level field(s) changed — add those + * to `normalizeRuleset` (`src/engine/dynamic.ts`) rather than to this test. + * + * Doubly gated like the permissions write round-trip (`tests/permission.integration.test.ts`): + * CT_LIVE=1 AND CT_LIVE_WRITE=1, plus an explicit CT_LIVE_WRITE_HOST host-match guard, so this can + * never fire against production by accident. It does not run in this repo's default CI or + * dev-machine state. + * + * PRECONDITION: CT_DYNAMIC_FIXTURE_GID must point at a DISPOSABLE dynamic group on a **dev** + * instance — one whose ruleset this test may freely overwrite. The test captures the group's + * current ruleset before writing and restores it in a `finally`, but a process kill / crash mid-run + * would leave the group holding the test-authored ruleset — never point this at a group anyone + * depends on. See docs/dynamic-groups.md and tests/fixtures/dynamic/README.md for how to run this. + */ +describe.runIf(live && liveWrite)("dynamic ruleset round-trip pin (#36, live write)", () => { + it("a user-authored PUT round-trips through GET, and a plan built from it is a no-op", async () => { + const { client } = await authedSession(); + + const expectedHost = process.env.CT_LIVE_WRITE_HOST?.trim(); + if (!expectedHost || client.host !== expectedHost) { + throw new Error( + "CT_LIVE_WRITE requires CT_LIVE_WRITE_HOST to be set and to exactly match the authenticated " + + "host, as a non-production confirmation guard. Refusing to write.", + ); + } + if (!GID) { + throw new Error("CT_DYNAMIC_FIXTURE_GID must be set to a disposable dev dynamic group id."); + } + + // Capture the group's current ruleset so it can be restored no matter what this test does. + const before = await client.get>(`/dynamicgroups/${GID}/ruleset`); + + try { + // Authored by hand, right here — NOT copied from a GET — so a server-side rewrite of any of + // these fields is actually observable. + const authored = { + description: `ct-cli #36 pin ${Date.now()}`, + shorty: "ct-cli-36-pin", + personIdFieldName: "person.id", + importance: 7, + query: { + method: "ChurchQuery", + params: { + groupBy: ["person.id"], + filter: { "==": [{ var: "person.isArchived" }, false] }, + primaryEntityAlias: "person", + responseFields: ["person.id"], + }, + }, + process: {}, + }; + + await client.request("PUT", `/dynamicgroups/${GID}/ruleset`, { dynamicGroupRuleSet: authored }); + const after = await client.get>(`/dynamicgroups/${GID}/ruleset`); + + const wantNorm = normalizeRuleset(authored); + const gotNorm = normalizeRuleset(after); + + // Field-by-field, not a single blob compare, so a failure names exactly what CT rewrote. + const allFields = new Set([...Object.keys(wantNorm), ...Object.keys(gotNorm)]); + const changedFields = [...allFields].filter((f) => !isDeepStrictEqual(wantNorm[f], gotNorm[f])); + if (changedFields.length > 0) { + const detail = changedFields + .map((f) => ` ${f}:\n authored: ${JSON.stringify(wantNorm[f])}\n returned: ${JSON.stringify(gotNorm[f])}`) + .join("\n"); + throw new Error( + `CT rewrote ${changedFields.length} RuleSet field(s) on PUT — extend normalizeRuleset ` + + `(src/engine/dynamic.ts) to drop/canonicalize them:\n${detail}`, + ); + } + + // The property #36 actually protects: a `ct plan` built from this same user-authored desired + // ruleset is a no-op (empty `dynamic` diff) after the PUT — not just raw-object equality. + // The `dynamic` field bundles status + ruleset and `diffFields` compares it as one unit, so the + // desired status must track the group's REAL live status rather than a hardcoded literal — + // otherwise this assertion would false-fail whenever the fixture group's status isn't exactly + // "manual" (e.g. the committed fixture at tests/fixtures/dynamic/status.get.json is "active"). + // This test only ever writes the ruleset (never a status PUT), so reading the live status here + // isolates exactly the #36 property under test — ruleset field rewriting — from status drift. + const liveStatus = ( + await client.get<{ dynamicGroupStatus?: string }>(`/dynamicgroups/${GID}/status`) + )?.dynamicGroupStatus ?? "none"; + const state: State = { + version: 1, + host: expectedHost, + resources: { pin36: { type: "group", id: GID, key: "pin36", fields: {}, adoptedAt: "t", updatedAt: "t" } }, + }; + const actual = new Map>([["pin36", {}]]); + const desired: DesiredResource[] = [ + { + type: "group", + key: "pin36", + fields: {}, + dependsOn: [], + dynamic: { status: liveStatus as DynamicStatus, ruleset: authored }, + }, + ]; + const folded = await syntheticField("dynamic")!.fold({ client, state, desired, actual }); + expect(folded.errors).toEqual([]); + const dynamicChange = diffFields(folded.desired[0]!.fields, actual.get("pin36")!).find( + (c) => c.field === "dynamic", + ); + expect(dynamicChange, `plan is not a no-op after the PUT: ${JSON.stringify(dynamicChange)}`).toBeUndefined(); + } finally { + // Restore the prior ruleset so the dev instance isn't left mutated by this test. + await client.request("PUT", `/dynamicgroups/${GID}/ruleset`, { dynamicGroupRuleSet: normalizeRuleset(before) }); + } + }); +}); diff --git a/tests/fixtures/dynamic/README.md b/tests/fixtures/dynamic/README.md index 6201c72..70c1603 100644 --- a/tests/fixtures/dynamic/README.md +++ b/tests/fixtures/dynamic/README.md @@ -27,3 +27,41 @@ were made to produce these; they are `GET` responses saved verbatim. The write-side round-trip test (plan Task 6) is gated behind `CT_LIVE=1` and a `CT_DYNAMIC_FIXTURE_GID` and must target a **dev** instance — **never run it against this production login.** + +## Pinned assumptions / how to run the gated round-trip (#36) + +`tests/dynamic.integration.test.ts` has three live-gated tests across two +`describe` blocks, all skipped by default (`npm test` never runs them): + +1. **`CT_LIVE=1`** — read → normalize → write-back-of-CT's-own-GET is a no-op, + and the committed fixture still matches the instance. Read-mostly; the + write-back re-sends exactly what CT returned, so it is drift-free by + construction. +2. **`CT_LIVE=1 CT_LIVE_WRITE=1 CT_LIVE_WRITE_HOST= + CT_DYNAMIC_FIXTURE_GID=`** — the **#36 pin**: + PUTs a ruleset this test authors itself (custom `description`, `shorty`, + `importance`, …, never copied from a GET) to the designated group, GETs it + back, and asserts normalized deep-equality field by field. It also builds + a plan from the same user-authored desired ruleset and asserts the + `dynamic` field diffs to no-op — the property #36 actually protects (a + no-op plan requires `deepEqual(desired.dynamic, actual.dynamic)`). The + group's prior ruleset is captured before the PUT and restored in a + `finally`. + +**What this pins:** that CT does not silently rewrite/normalize a +RuleSet-level field (`description`, `shorty`, `importance`, +`personIdFieldName`, `process`) on `PUT` — `normalizeRuleset` +(`src/engine/dynamic.ts`) only strips the two read-only timestamp keys and +normalizes the `query` subtree; it currently assumes every other RuleSet-level +field round-trips byte-for-byte. + +**If it fails:** the failure message names exactly which RuleSet-level +field(s) CT rewrote and shows authored-vs-returned values. Extend +`normalizeRuleset` to canonicalize/drop those fields the same way it already +handles `query` — do not weaken the test. + +**Precondition:** `CT_DYNAMIC_FIXTURE_GID` must point at a **disposable** +dynamic group on a **dev** instance whose ruleset may be freely overwritten. +The test restores the group's prior ruleset afterward, but a process kill or +crash mid-run would leave it holding the test-authored ruleset — never point +this at a group anyone depends on, and never at the production login.