Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions docs/dynamic-groups.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
126 changes: 126 additions & 0 deletions tests/dynamic.integration.test.ts
Original file line number Diff line number Diff line change
@@ -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)", () => {
Expand All @@ -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<Record<string, unknown>>(`/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<Record<string, unknown>>(`/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<string, Record<string, unknown>>([["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) });
}
});
});
38 changes: 38 additions & 0 deletions tests/fixtures/dynamic/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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=<dev host>
CT_DYNAMIC_FIXTURE_GID=<disposable dev group id>`** — 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.
Loading