fix: handle ChurchTools group children responses

docs/handbuch/dynamic-groups.md

@@ -6,7 +6,7 @@ sources:
   - src/engine/dynamic.ts
   - src/engine/synthetic.ts
   - src/commands/adopt-group.ts
-sources_hash: d67f76530e841833
+sources_hash: 13e4a54533b0e8a2
 reviewed: 2026-08-17
 ---
 

src/commands/adopt-group.ts

@@ -101,28 +101,66 @@ async function resolveGroupId(
   return Number(candidates[0]!.id);
 }
 
+/**
+ * Resolve one `/groups/{id}/children` row to the group id it points at.
+ *
+ * CT answers this endpoint with either plain group rows (`{ id }`) or domain resources
+ * (`{ domainType: "group", domainIdentifier, apiUrl }`). For a domain resource the authoritative
+ * group id is `domainIdentifier` — a sibling `id`, if CT ever emits one, is the hierarchy edge's
+ * own id — so `domainIdentifier` is read first and `id` only backs it up. `apiUrl` is the last
+ * resort. `parentId` is threaded in purely so the error names the group that actually failed:
+ * `--children-of` walks whole subtrees, and "some group somewhere" is not actionable.
+ */
+function childId(raw: unknown, parentId: number): number {
+  let candidate: unknown = raw;
+  if (raw !== null && typeof raw === "object") {
+    const child = raw as Record<string, unknown>;
+    candidate = child.domainIdentifier ?? child.id;
+    if (candidate == null && typeof child.apiUrl === "string") {
+      candidate = /\/groups\/(\d+)(?:[/?#]|$)/.exec(child.apiUrl)?.[1];
+    }
+  }
+
+  const id =
+    typeof candidate === "number"
+      ? candidate
+      : typeof candidate === "string" && /^\d+$/.test(candidate.trim())
+        ? Number.parseInt(candidate, 10)
+        : Number.NaN;
+  if (!Number.isSafeInteger(id) || id < 0) {
+    const shape =
+      raw !== null && typeof raw === "object"
+        ? `{ ${Object.keys(raw as Record<string, unknown>).join(", ")} }`
+        : JSON.stringify(raw);
+    throw new Error(
+      `GET /groups/${parentId}/children returned a child without a usable id (${shape}); ` +
+        `expected a number or an object with id, domainIdentifier, or apiUrl.`,
+    );
+  }
+  return id;
+}
+
 /**
  * Recursively collect a group's full hierarchy subtree via `/groups/{id}/children`, in
  * parent-before-child (pre-order) sequence, excluding the root itself. Guards against a cyclic
  * hierarchy (a live-API bug, not a valid DAG state) with a `visited` set — never re-descends into
  * an id already seen, so a back-reference to an ancestor cannot loop forever.
+ *
+ * `/groups/{id}/children` is a paginated list endpoint, so it is read with `getAll`, never a plain
+ * `get` (#101): a plain GET returns only CT's default first page, which would silently drop the
+ * tail of a wide Bereich and every subtree hanging off it. `getAll` also absorbs CT's inconsistent
+ * page shapes (bare array vs. `{ data: [...] }`) and an empty 204 body, which for a leaf group is
+ * simply "no children".
  */
-async function collectSubtreeIds(rootId: number, client: Pick<CtClient, "get">): Promise<number[]> {
+async function collectSubtreeIds(rootId: number, client: Pick<CtClient, "getAll">): Promise<number[]> {
   const visited = new Set<number>([rootId]);
   const order: number[] = [];
 
   async function walk(id: number): Promise<void> {
-    let raw: unknown;
-    try {
-      raw = await client.get(`/groups/${id}/children`);
-    } catch (err) {
-      if (err instanceof CtApiError && err.status === 404) return; // no children (leaf, or unknown group)
-      throw err;
-    }
-    const children = Array.isArray(raw) ? raw : [];
-    for (const c of children) {
-      const cid = typeof c === "number" ? c : Number((c as Record<string, unknown> | null)?.id);
-      if (!Number.isFinite(cid) || visited.has(cid)) continue;
+    const page = await client.getAll<unknown>(`/groups/${id}/children`);
+    for (const c of page.data) {
+      const cid = childId(c, id);
+      if (visited.has(cid)) continue;
       visited.add(cid);
       order.push(cid);
       await walk(cid);

tests/adopt-group-command.test.ts

@@ -15,7 +15,9 @@ import type { CtClient } from "../src/api/ctClient.js";
  *  - `/group/grouptypes` (for --type's logical-key resolution)
  *  - `/dynamicgroups/{id}/ruleset` + `/status` (for --with-dynamic; #31 is deliberately NOT dynamic)
  */
-function makeClient() {
+type ChildrenResponse = "array" | "envelope" | "domain-envelope" | "root-404";
+
+function makeClient(childrenResponse: ChildrenResponse = "array") {
   const groups: Record<number, Record<string, unknown>> = {
     10: { id: 10, name: "Area A", information: { groupTypeId: 5, groupStatusId: 1 } },
     11: { id: 11, name: "Area B", information: { groupTypeId: 5, groupStatusId: 1 } },
@@ -79,7 +81,24 @@ function makeClient() {
       return g;
     }
     m = /^\/groups\/(\d+)\/children$/.exec(path);
-    if (m) return (children[Number(m[1])] ?? []).map((id) => ({ id }));
+    if (m) {
+      const parentId = Number(m[1]);
+      if (childrenResponse === "root-404" && parentId === 40) {
+        throw new CtApiError("not found", 404, null);
+      }
+      const rows = (children[parentId] ?? []).map((id) => ({ id }));
+      if (childrenResponse === "envelope") return { data: rows };
+      if (childrenResponse === "domain-envelope") {
+        return {
+          data: rows.map(({ id }) => ({
+            domainIdentifier: String(id),
+            domainType: "group",
+            apiUrl: `/groups/${id}`,
+          })),
+        };
+      }
+      return rows;
+    }
     if (path === "/group/grouptypes") return groupTypes;
     if (path === "/campuses") return campuses;
     if (path === "/group/roles") return roles;
@@ -97,10 +116,13 @@ function makeClient() {
 
   const getAll = vi.fn(async (path: string) => {
     if (path === "/groups") return { data: Object.values(groups) };
-    // The Resolver reads master-data catalogs paginated (#99 review), so serve them here too —
-    // same rows as `get`, wrapped in the page envelope.
+    // The Resolver reads master-data catalogs paginated (#99 review), and `--children-of` reads
+    // `/groups/{id}/children` paginated (#101), so serve them here too — same rows as `get`, with
+    // the real client's envelope normalization (bare array or `{ data: [...] }` -> page items).
     const single = await get(path);
-    return { data: Array.isArray(single) ? single : [single] };
+    if (Array.isArray(single)) return { data: single };
+    const inner = (single as { data?: unknown }).data;
+    return { data: Array.isArray(inner) ? inner : [single] };
   });
 
   return { get, getAll };
@@ -246,6 +268,10 @@ describe("ct adopt group --children-of", () => {
     // 41 (child of root) must be adopted before 43 (child of 41).
     const order = Object.values(state.resources).map((r) => r.id);
     expect(order.indexOf(41)).toBeLessThan(order.indexOf(43));
+    // `/groups/{id}/children` is a paginated list endpoint: read via `getAll`, never a plain `get`
+    // (#101), or a wide Bereich silently loses everything past CT's default first page.
+    expect(client.getAll).toHaveBeenCalledWith("/groups/40/children");
+    expect(client.getAll).toHaveBeenCalledWith("/groups/41/children");
   });
 
   it("terminates on a cyclic hierarchy instead of looping forever (cycle guard)", async () => {
@@ -255,6 +281,29 @@ describe("ct adopt group --children-of", () => {
     expect(ids).toEqual([51]); // 50 -> 51 -> 50: only 51 is a new descendant
   });
 
+  it("accepts the raw { data: [...] } collection envelope", async () => {
+    client = makeClient("envelope");
+    await run(["group", "--children-of", "40", "--state", statePath]);
+
+    const state = await loadState(statePath, HOST);
+    expect(Object.values(state.resources).map((r) => r.id)).toEqual([41, 43, 42]);
+  });
+
+  it("reads ids from the domain resources documented by the live ChurchTools OpenAPI", async () => {
+    client = makeClient("domain-envelope");
+    await run(["group", "--children-of", "40", "--state", statePath]);
+
+    const state = await loadState(statePath, HOST);
+    expect(Object.values(state.resources).map((r) => r.id)).toEqual([41, 43, 42]);
+  });
+
+  it("propagates a children endpoint 404 instead of treating the group as a leaf", async () => {
+    client = makeClient("root-404");
+    await expect(run(["group", "--children-of", "40", "--state", statePath])).rejects.toMatchObject({
+      status: 404,
+    });
+  });
+
   it("resolves --children-of by an already-adopted state key", async () => {
     await run(["group", "40", "--state", statePath]); // adopt the root first, under its derived key "root"
     await run(["group", "--children-of", "root", "--state", statePath]);