diff --git a/src/commands/apply.ts b/src/commands/apply.ts index 897e020..ffd7738 100644 --- a/src/commands/apply.ts +++ b/src/commands/apply.ts @@ -15,6 +15,7 @@ import { buildPermissionPlan } from "../permissions/plan.js"; import { renderPermissionPlan } from "../permissions/render.js"; import { applyPermissionPlan } from "../permissions/apply.js"; import { confirm } from "../ui/prompt.js"; +import { resolveWithEnv } from "../util/resolve.js"; import { info, warn, success, error } from "../ui.js"; interface ApplyOptions { @@ -74,7 +75,7 @@ export function resolveBackupDir( statePath: string, env: NodeJS.ProcessEnv = process.env, ): string { - return explicit?.trim() || env.CT_BACKUP_DIR?.trim() || join(dirname(statePath), "backups"); + return resolveWithEnv(explicit, env.CT_BACKUP_DIR, join(dirname(statePath), "backups")); } export function applyCommand(): Command { diff --git a/src/commands/destroy.ts b/src/commands/destroy.ts index afc1374..fad584e 100644 --- a/src/commands/destroy.ts +++ b/src/commands/destroy.ts @@ -3,10 +3,12 @@ import { authedSession } from "../api/session.js"; import { CtApiError, type CtClient } from "../api/ctClient.js"; import { resolveConfig } from "../config.js"; import { loadState, resolveStatePath, saveState, type State } from "../state/state.js"; -import { loadConfig, resolveConfigPath } from "../config/load.js"; import { RESOURCES } from "../resources/registry.js"; import { assertNotPeople } from "../engine/guard.js"; -import { tierOf } from "../engine/graph.js"; +import { orderKeys } from "../engine/graph.js"; +import { fetchActual } from "../engine/build.js"; +import { parentIdsByGroupId, managedParentKeys, type HierarchyEntry } from "../engine/hierarchy.js"; +import type { DesiredResource } from "../engine/types.js"; import { writeBackup } from "../engine/backup.js"; import { resolveBackupDir } from "./apply.js"; import { confirmTyped } from "../ui/prompt.js"; @@ -14,7 +16,6 @@ import { info, warn, success, error } from "../ui.js"; interface DestroyOptions { target?: string[]; - config?: string; state?: string; backupDir?: string; force?: boolean; @@ -34,16 +35,69 @@ export function parseTargets(raw: string[]): string[] { return out; } -/** Reverse dependency order: highest tier first (leaves before their base metadata). */ -export function orderDestroy(state: State, keys: string[]): string[] { - return [...keys].sort((a, b) => tierOf(state.resources[b]!.type) - tierOf(state.resources[a]!.type)); +/** + * Reverse dependency order for destroy: highest tier first (leaves before their + * base metadata) and, within the group tier, a child before its parent. + * + * The state file carries no hierarchy edges (the synthetic `parents` field is + * stripped from snapshots — see execute.ts), so the caller passes `parentKeysByKey` + * discovered live from `/groups/hierarchies` (managed groups only). We reuse + * `orderKeys` — the very topological apply order plan uses — with those edges, + * then reverse it, so destroy is the exact inverse of apply and honours intra-tier + * parent edges. Pass an empty map (or omit) to fall back to tier-only ordering. + */ +export function orderDestroy( + state: State, + keys: string[], + parentKeysByKey: Map = new Map(), +): string[] { + const entries: DesiredResource[] = keys.map((key) => ({ + type: state.resources[key]!.type, + key, + fields: {}, + dependsOn: parentKeysByKey.get(key) ?? [], + })); + return orderKeys(entries).reverse(); +} + +/** + * Discover managed group→parent edges from the live `/groups/hierarchies`, so + * `orderDestroy` can put a child before its parent. Only the group targets need + * edges; every managed group is mapped id→key so a parent edge to a not-targeted + * managed group is still resolvable (harmless — `orderKeys` ignores deps outside + * the target set). Best-effort: a fetch failure warns and returns no edges, so + * ordering degrades to tier-only rather than aborting the destroy. + */ +async function fetchParentEdges( + client: Pick, + state: State, + keys: string[], +): Promise> { + const groupKeys = keys.filter((k) => state.resources[k]?.type === "group"); + if (groupKeys.length === 0) return new Map(); + const groupIdToKey = new Map(); + for (const m of Object.values(state.resources)) { + if (m.type === "group") groupIdToKey.set(m.id, m.key); + } + try { + const raw = await client.get("/groups/hierarchies"); + const parentIds = parentIdsByGroupId(Array.isArray(raw) ? raw : []); + const edges = new Map(); + for (const key of groupKeys) { + edges.set(key, managedParentKeys(parentIds.get(state.resources[key]!.id) ?? [], groupIdToKey)); + } + return edges; + } catch (err) { + const message = err instanceof Error ? err.message : String(err); + warn(`Failed to fetch group hierarchies for destroy ordering: ${message}. Falling back to tier-only order.`); + return new Map(); + } } export function destroyCommand(): Command { return new Command("destroy") .description("Explicitly delete managed resources (protected; never implicit)") .requiredOption("--target ", "logical key(s) to destroy (repeatable or comma-separated)") - .option("-c, --config ", "config file (or set CT_CONFIG)") .option("-s, --state ", "state file (or set CT_STATE)") .option("--backup-dir ", "directory for the pre-destroy backup (or set CT_BACKUP_DIR)") .option("--force", "skip the typed confirmation (preventDestroy is still enforced)") @@ -63,35 +117,35 @@ export function destroyCommand(): Command { } } - // preventDestroy guard: a target still declared with the flag is blocked. - const { resources: desired } = await loadConfig(resolveConfigPath(opts.config)); - const protectedKeys = new Set(desired.filter((d) => d.preventDestroy).map((d) => d.key)); - const blocked = targets.filter((k) => protectedKeys.has(k)); + // preventDestroy guard: read from STATE, never the config. A resource dropped from config + // (the real destroy scenario) has lost its config flag, but its state entry still carries the + // protection apply mirrored there — so it survives the drop. destroy loads no config file at + // all, so a config eval error (e.g. a sibling still referencing the dropped target) can't + // block a teardown either (items 2 + 3). + const blocked = targets.filter((k) => state.resources[k]!.preventDestroy); if (blocked.length > 0) { throw new Error( - `preventDestroy is set for: ${blocked.join(", ")}. Remove the flag in config first.`, + `preventDestroy is set (in state) for: ${blocked.join(", ")}. ` + + `Set preventDestroy:false in config and re-apply (or clear it in the state file) first.`, ); } - const ordered = orderDestroy(state, targets); const { client } = await authedSession(); - // Backup: fetch each target's current actual values (best-effort; 404 → skip). - const actual = new Map>(); - for (const key of ordered) { - const managed = state.resources[key]!; - const spec = RESOURCES[managed.type]; - if (!spec) { - continue; - } - try { - const raw = await client.get>(spec.itemPath(managed.id)); - actual.set(key, spec.managedFields(raw)); - } catch (err) { - if (!(err instanceof CtApiError && err.status === 404)) { - throw err; - } - } + const parentEdges = await fetchParentEdges(client, state, targets); + const ordered = orderDestroy(state, targets, parentEdges); + + // Backup: fetch each target's current actual values via the same fetchActual as plan/apply + // (404 → skip: already gone in CT, nothing to back up). A non-404 failure must ABORT before + // any DELETE — proceeding would irreversibly delete a target with no backup of its state. + const { actual, fetchErrors } = await fetchActual(client, ordered.map((k) => state.resources[k]!)); + if (fetchErrors.length > 0) { + error( + `Backup fetch failed for: ${fetchErrors.join("; ")}. ` + + `Nothing was deleted — resolve the error (or wait out the outage) and re-run.`, + ); + process.exitCode = 1; + return; } const backupPath = await writeBackup(resolveBackupDir(opts.backupDir, statePath), config.host, actual); info(`Backup written: ${backupPath}`); diff --git a/src/config/context.ts b/src/config/context.ts index f5d148d..e3d1e77 100644 --- a/src/config/context.ts +++ b/src/config/context.ts @@ -32,7 +32,11 @@ export interface ResourceInput { */ parents?: string[]; dependsOn?: string[]; - /** Block `ct destroy` for this resource until the flag is removed. */ + /** + * Block `ct destroy` for this resource. Mirrored to the state entry on `apply`, + * so protection survives the resource being dropped from config; clear it by + * setting `false` (or removing it) and re-applying before you destroy. + */ preventDestroy?: boolean; [field: string]: unknown; } @@ -89,7 +93,9 @@ function toDesired(type: string, input: ResourceInput): DesiredResource { // `parent` is an ordering hint only — a dependency edge, never a diffed/managed field // (its pre-hierarchy meaning; a `parent` may point at a campus). Group hierarchy is // managed opt-in via `parents`: `undefined` → unmanaged, `[]` → managed with no parents. - const parentKey = typeof parent === "string" && parent !== "" ? parent : undefined; + // `parent` is already narrowed to `string | null | undefined` by the guard above; `|| undefined` + // collapses null and "" (an empty parent is "no parent", never opt-in) to undefined. + const parentKey = parent || undefined; const parentKeys = parents !== undefined ? [...new Set(parents)] : undefined; const edges = [...new Set([...dependsOn, ...(parentKey ? [parentKey] : []), ...(parentKeys ?? [])])]; return { diff --git a/src/config/load.ts b/src/config/load.ts index ba12dc2..3328701 100644 --- a/src/config/load.ts +++ b/src/config/load.ts @@ -12,11 +12,12 @@ import { createJiti } from "jiti"; import { evaluateConfig, type ConfigModule } from "./context.js"; import type { DesiredResource } from "../engine/types.js"; import type { DesiredPermission } from "../permissions/types.js"; +import { resolveWithEnv } from "../util/resolve.js"; export const DEFAULT_CONFIG_PATH = "ct.config.ts"; export function resolveConfigPath(explicit?: string, env: NodeJS.ProcessEnv = process.env): string { - return explicit?.trim() || env.CT_CONFIG?.trim() || DEFAULT_CONFIG_PATH; + return resolveWithEnv(explicit, env.CT_CONFIG, DEFAULT_CONFIG_PATH); } export async function loadConfig( diff --git a/src/engine/backup.ts b/src/engine/backup.ts index f0e4b88..cfb6db2 100644 --- a/src/engine/backup.ts +++ b/src/engine/backup.ts @@ -5,6 +5,23 @@ */ import { mkdir, writeFile } from "node:fs/promises"; import { join } from "node:path"; +import { isSyntheticField } from "./synthetic.js"; + +/** + * Drop synthetic pseudo-fields (`parents`, `dynamic`, …) from an actual record. + * + * `buildPlan` folds these into its `actual` map IN PLACE for diffing, and apply + * reuses that same map for the backup. They are internal logical-key sets, not + * real CT columns, so they are non-restorable noise in a backup — strip them so + * the file holds only real, restorable values (and matches destroy's clean backup). + */ +function realFields(fields: Record): Record { + const out: Record = {}; + for (const [k, v] of Object.entries(fields)) { + if (!isSyntheticField(k)) out[k] = v; + } + return out; +} export async function writeBackup( dir: string, @@ -18,7 +35,7 @@ export async function writeBackup( const payload = { host, capturedAt: now.toISOString(), - resources: Object.fromEntries(actual), + resources: Object.fromEntries([...actual].map(([key, fields]) => [key, realFields(fields)])), }; await writeFile(path, `${JSON.stringify(payload, null, 2)}\n`, "utf8"); return path; diff --git a/src/engine/build.ts b/src/engine/build.ts index ee9a4b5..56e6200 100644 --- a/src/engine/build.ts +++ b/src/engine/build.ts @@ -7,7 +7,7 @@ */ import type { CtClient } from "../api/ctClient.js"; import { CtApiError } from "../api/ctClient.js"; -import type { State } from "../state/state.js"; +import type { ManagedResource, State } from "../state/state.js"; import type { DesiredResource, Plan } from "./types.js"; import { RESOURCES } from "../resources/registry.js"; import { computePlan } from "./plan.js"; @@ -24,25 +24,36 @@ export interface BuildResult { fetchErrors: string[]; } -export interface BuildOptions { - /** Directory of the config file — `{ ref }` ruleset paths resolve relative to it (not the cwd). */ - configDir?: string; +export interface FetchActualResult { + /** Managed fields per logical key, for every resource that fetched cleanly (a 404 is omitted, not an error). */ + actual: Map>; + /** Keys whose managed type has no registry entry — cannot be fetched/diffed, so left untouched. */ + unresolved: Set; + /** Keys whose fetch errored (non-404), mapped to a short status descriptor for the plan render. */ + fetchFailed: Map; + /** Human-readable fetch-error lines (non-404), one per failed key. */ + fetchErrors: string[]; } -export async function buildPlan( +/** + * Fetch the actual ChurchTools values of a set of managed resources, concurrently. + * + * Shared by `buildPlan` (plan/apply) and `destroy` (its pre-delete backup) so the + * two read actuals identically and cannot drift. A 404 means the resource vanished + * in CT — omitted from `actual` (the plan recreates/prunes; the backup skips it). + * A non-404 error is recorded (never thrown) so one bad fetch neither aborts a + * read-only plan nor blocks tearing down the other targets. + */ +export async function fetchActual( client: Pick, - state: State, - desired: DesiredResource[], - opts: BuildOptions = {}, -): Promise { - // Keyed by logical key (globally unique), not CT id (unique only within a type — the Mainz campus is id 0). + resources: readonly ManagedResource[], +): Promise { const actual = new Map>(); const unresolved = new Set(); - // Keys whose fetch errored (non-404), mapped to a short status descriptor for the plan render. const fetchFailed = new Map(); const fetchErrors: string[] = []; - await mapConcurrent(Object.values(state.resources), FETCH_CONCURRENCY, async (managed) => { + await mapConcurrent(resources, FETCH_CONCURRENCY, async (managed) => { const spec = RESOURCES[managed.type]; if (!spec) { unresolved.add(managed.key); @@ -68,6 +79,26 @@ export async function buildPlan( } }); + return { actual, unresolved, fetchFailed, fetchErrors }; +} + +export interface BuildOptions { + /** Directory of the config file — `{ ref }` ruleset paths resolve relative to it (not the cwd). */ + configDir?: string; +} + +export async function buildPlan( + client: Pick, + state: State, + desired: DesiredResource[], + opts: BuildOptions = {}, +): Promise { + // Keyed by logical key (globally unique), not CT id (unique only within a type — the Mainz campus is id 0). + const { actual, unresolved, fetchFailed, fetchErrors } = await fetchActual( + client, + Object.values(state.resources), + ); + // Synthetic sub-resource fields (parents, dynamic, …) fold into the diff on both sides. const folded = await foldSynthetic({ client, state, desired, actual, configDir: opts.configDir }); fetchErrors.push(...folded.errors); diff --git a/src/engine/execute.ts b/src/engine/execute.ts index 5b07c2b..91c8b46 100644 --- a/src/engine/execute.ts +++ b/src/engine/execute.ts @@ -9,7 +9,7 @@ * their own dedicated endpoints, not the owning resource's body — see synthetic.ts. */ import type { CtClient } from "../api/ctClient.js"; -import type { State } from "../state/state.js"; +import type { ManagedResource, State } from "../state/state.js"; import { upsert, saveState } from "../state/state.js"; import type { FieldChange, Plan } from "./types.js"; import { RESOURCES } from "../resources/registry.js"; @@ -31,6 +31,12 @@ export interface ExecuteResult { failed?: { key: string; message: string }; } +/** Mirror the config's `preventDestroy` onto a state entry (kept absent, not `false`, when unset). */ +function mirrorPreventDestroy(entry: ManagedResource, flag: boolean | undefined): void { + if (flag) entry.preventDestroy = true; + else delete entry.preventDestroy; +} + /** The managed field snapshot after a write: base ∪ changed fields, minus any synthetic sub-resource fields. */ function snapshotFromChanges(base: Record, changes: FieldChange[]): Record { const snap = { ...base }; @@ -62,6 +68,15 @@ export async function executePlan(plan: Plan, deps: ExecuteDeps): Promise d.type === "group" && d.parents !== undefined).map((d) => d.key), - ); - - for (const managed of Object.values(state.resources)) { - if (managed.type !== "group" || !optedIn.has(managed.key)) { - continue; - } - const a = actual.get(managed.key); - if (a) { + // For each opted-in desired group that already exists in state, set its actual `parents` to the + // managed parent keys — mirroring the desired-side guard below (one pass, no separate opted-in set). + // A group not yet in state (fresh, first apply) has no actual to annotate; it is created instead. + for (const d of desired) { + if (d.type !== "group" || d.parents === undefined) continue; + const managed = state.resources[d.key]; + const a = managed && actual.get(d.key); + if (managed && a) { a.parents = managedParentKeys(parentIdsByGroup.get(managed.id) ?? [], groupIdToKey); } } diff --git a/src/engine/plan.ts b/src/engine/plan.ts index 9c4b091..12e70c8 100644 --- a/src/engine/plan.ts +++ b/src/engine/plan.ts @@ -111,6 +111,7 @@ export function computePlan( id: null, action: "create", changes: diffFields(d.fields, {}), + preventDestroy: d.preventDestroy, }); continue; } @@ -129,6 +130,7 @@ export function computePlan( action: "no-op", changes: [], note: "unresolved-type", + preventDestroy: d.preventDestroy, }); continue; } @@ -142,6 +144,7 @@ export function computePlan( changes: [], note: "fetch-failed", detail: fetchFailed.get(d.key), + preventDestroy: d.preventDestroy, }); continue; } @@ -154,6 +157,7 @@ export function computePlan( action: "create", changes: diffFields(d.fields, {}), note: "recreate", + preventDestroy: d.preventDestroy, }); continue; } @@ -168,6 +172,7 @@ export function computePlan( // The fetched actual — the write body is built from this, not the stale state snapshot (#27). actual: a, drift: drift.length > 0 ? drift : undefined, + preventDestroy: d.preventDestroy, }); } diff --git a/src/engine/types.ts b/src/engine/types.ts index 19b7e62..c109717 100644 --- a/src/engine/types.ts +++ b/src/engine/types.ts @@ -66,6 +66,12 @@ export interface PlanItem { note?: PlanNote; /** Extra human-readable context for a note (e.g. the HTTP status behind `fetch-failed`). */ detail?: string; + /** + * The config's `preventDestroy` for this desired resource, carried so apply can + * mirror it onto the state entry (state, not config, guards `destroy`). Only set + * on desired-side items; undefined on delete-side items. + */ + preventDestroy?: boolean; } export interface Plan { diff --git a/src/state/state.ts b/src/state/state.ts index cfad233..c202ea0 100644 --- a/src/state/state.ts +++ b/src/state/state.ts @@ -13,6 +13,7 @@ * uses explicit null/undefined checks — never truthiness. */ import { readFile, writeFile } from "node:fs/promises"; +import { resolveWithEnv } from "../util/resolve.js"; export interface ManagedResource { type: string; @@ -22,6 +23,13 @@ export interface ManagedResource { fields: Record; adoptedAt: string; updatedAt: string; + /** + * Lifecycle flag mirrored from the config's `preventDestroy` at apply time. + * State — not the ephemeral config — is the source of truth for destroy + * protection, so a resource stays protected after it is dropped from config + * (the exact moment it becomes a destroy candidate). Missing = not protected. + */ + preventDestroy?: boolean; } export interface State { @@ -34,7 +42,7 @@ export interface State { export const DEFAULT_STATE_PATH = "ct-state.json"; export function resolveStatePath(explicit?: string, env: NodeJS.ProcessEnv = process.env): string { - return explicit?.trim() || env.CT_STATE?.trim() || DEFAULT_STATE_PATH; + return resolveWithEnv(explicit, env.CT_STATE, DEFAULT_STATE_PATH); } export function emptyState(host: string): State { @@ -66,7 +74,7 @@ export async function loadState(path: string, host: string): Promise { throw new Error(`Malformed state file ${path}: not valid JSON (${(err as Error).message}).`); } - const state = validateState(parsed, path); + const state = migrateState(validateState(parsed, path)); if (state.host !== host) { throw new Error( `State file host (${state.host}) does not match CT_HOST (${host}). Refusing to mix instances.`, @@ -93,6 +101,29 @@ function validateState(parsed: unknown, path: string): State { return obj as unknown as State; } +/** + * In-place, version-preserving migrations for state loaded from disk. + * + * The campus registry field was renamed `shortName → shorty` (Phase 4) without a + * state-version bump, so a campus adopted before the rename carries a stale + * `shortName` key. Left alone it drifts forever (`shortName → undefined` on every + * plan) and a real update PUTs the vestigial `shortName` while omitting the + * create-required `shorty`. Rename the key on load — only when `shorty` is absent, + * so a post-rename snapshot is never clobbered. The next apply re-writes the real + * value; this just clears the phantom drift so the diff can converge. + */ +function migrateState(state: State): State { + for (const resource of Object.values(state.resources)) { + if (resource.type !== "campus") continue; + const fields = resource.fields; + if ("shortName" in fields && !("shorty" in fields)) { + fields.shorty = fields.shortName; + delete fields.shortName; + } + } + return state; +} + export async function saveState(path: string, state: State): Promise { await writeFile(path, `${JSON.stringify(state, null, 2)}\n`, "utf8"); } diff --git a/src/util/resolve.ts b/src/util/resolve.ts new file mode 100644 index 0000000..84dda48 --- /dev/null +++ b/src/util/resolve.ts @@ -0,0 +1,13 @@ +/** + * Shared precedence for a path/dir setting: an explicit flag wins, else an + * environment value, else a hardcoded fallback. Whitespace-only values are + * treated as unset (a bare `--flag ""` or `VAR=" "` falls through). Used by + * every `resolve` helper so the idiom lives once. + */ +export function resolveWithEnv( + explicit: string | undefined, + envValue: string | undefined, + fallback: string, +): string { + return explicit?.trim() || envValue?.trim() || fallback; +} diff --git a/tests/backup.test.ts b/tests/backup.test.ts index dc689a1..885132f 100644 --- a/tests/backup.test.ts +++ b/tests/backup.test.ts @@ -27,6 +27,22 @@ describe("writeBackup", () => { }); }); + it("strips synthetic pseudo-fields (parents, dynamic) so a backup holds only real values (#17 item 6)", async () => { + dir = await mkdtemp(join(tmpdir(), "ct-backup-test-")); + // buildPlan folds `parents`/`dynamic` into its actual map in place; apply reuses that same map. + const actual = new Map>([ + [ + "kids", + { name: "Kids", groupTypeId: 2, parents: ["area"], dynamic: { status: "active", ruleset: {} } }, + ], + ]); + const path = await writeBackup(dir, "h", actual, new Date("2026-01-01T00:00:00.000Z")); + const parsed = JSON.parse(await readFile(path, "utf8")); + expect(parsed.resources.kids).toEqual({ name: "Kids", groupTypeId: 2 }); + // The shared map itself is not mutated (executePlan may still read it after the backup). + expect(actual.get("kids")).toHaveProperty("parents"); + }); + it("creates the backup directory if it does not exist", async () => { const base = await mkdtemp(join(tmpdir(), "ct-backup-test-")); dir = base; diff --git a/tests/destroy-command.test.ts b/tests/destroy-command.test.ts new file mode 100644 index 0000000..216703e --- /dev/null +++ b/tests/destroy-command.test.ts @@ -0,0 +1,140 @@ +/** + * Command-level behaviour for `ct destroy` (#17 items 1–3), mocking only the + * session + backup so the real command wiring runs: + * - preventDestroy is read from STATE, and destroy loads NO config file at all + * (no loadConfig mock, no config on disk) — so protection survives dropping a + * resource from config, and a config eval error can never block a teardown. + * - delete ordering honours the live /groups/hierarchies edges: a child group is + * deleted before its parent even when --target lists the parent first. + */ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +interface Call { + method: string; + path: string; +} + +const calls: Call[] = []; +const getMock = vi.fn(async (path: string) => { + if (path === "/groups/hierarchies") { + return [ + { groupId: 1, parents: [] }, + { groupId: 2, parents: [1] }, // kids (2) → area (1) + ]; + } + return { name: path }; // itemPath backup fetch — any body +}); +const requestMock = vi.fn(async (method: string, path: string) => { + calls.push({ method, path }); + return {}; +}); + +vi.mock("../src/api/session.js", () => ({ + authedSession: vi.fn(async () => ({ client: { get: getMock, request: requestMock }, me: { id: 1 } })), +})); + +vi.mock("../src/engine/backup.js", () => ({ + writeBackup: vi.fn(async () => "backup.json"), +})); + +const { destroyCommand } = await import("../src/commands/destroy.js"); +const { saveState, loadState, emptyState } = await import("../src/state/state.js"); + +const statePath = join(tmpdir(), `ct-cli-destroy-cmd-${process.pid}.json`); +const HOST = "https://eqrm.church.tools"; +const originalHost = process.env.CT_HOST; + +function group(key: string, id: number, extra: Record = {}) { + return { type: "group", id, key, fields: {}, adoptedAt: "t", updatedAt: "t", ...extra }; +} + +async function runDestroy(args: string[]): Promise { + await destroyCommand().parseAsync(args, { from: "user" }); +} + +beforeEach(() => { + calls.length = 0; + requestMock.mockClear(); + getMock.mockClear(); + process.env.CT_HOST = HOST; +}); + +afterEach(async () => { + if (originalHost === undefined) delete process.env.CT_HOST; + else process.env.CT_HOST = originalHost; + await rm(statePath, { force: true }); +}); + +describe("ct destroy (command level)", () => { + it("blocks a target protected in STATE — with no config file loaded at all (#17 items 2+3)", async () => { + const state = emptyState(HOST); + state.resources.area = group("area", 1, { preventDestroy: true }); + await saveState(statePath, state); + + await expect(runDestroy(["--target", "area", "--state", statePath, "--force"])).rejects.toThrow( + /preventDestroy is set \(in state\) for: area/, + ); + // Nothing was deleted, and no config was consulted (the mock set has no loadConfig). + expect(requestMock).not.toHaveBeenCalled(); + }); + + it("deletes a child before its parent using live hierarchy edges, ignoring --target order (#17 item 1)", async () => { + const state = emptyState(HOST); + state.resources.area = group("area", 1); + state.resources.kids = group("kids", 2); + await saveState(statePath, state); + + // --target lists the PARENT first; a tier-only order would DELETE /groups/1 while kids still refs it. + await runDestroy(["--target", "area,kids", "--state", statePath, "--force"]); + + const deletes = calls.filter((c) => c.method === "DELETE").map((c) => c.path); + expect(deletes).toEqual(["/groups/2", "/groups/1"]); // child (kids #2) before parent (area #1) + const after = await loadState(statePath, HOST); + expect(after.resources).toEqual({}); + }); + + it("aborts before any DELETE when a target's backup fetch fails with a non-404", async () => { + const state = emptyState(HOST); + state.resources.area = group("area", 1); + state.resources.kids = group("kids", 2); + await saveState(statePath, state); + + const { CtApiError } = await import("../src/api/ctClient.js"); + getMock.mockImplementation(async (path: string) => { + if (path === "/groups/hierarchies") { + return [ + { groupId: 1, parents: [] }, + { groupId: 2, parents: [1] }, + ]; + } + if (path === "/groups/2") throw new CtApiError("Server Error", 500, null); + return { name: path }; + }); + const originalExitCode = process.exitCode; + + try { + await runDestroy(["--target", "area,kids", "--state", statePath, "--force"]); + + // The 500 on kids' backup fetch must abort the whole run before any DELETE: + // deleting an unbacked-up target is exactly what the pre-destroy backup guards against. + expect(requestMock).not.toHaveBeenCalled(); + expect(process.exitCode).toBe(1); + const after = await loadState(statePath, HOST); + expect(Object.keys(after.resources).sort()).toEqual(["area", "kids"]); + } finally { + process.exitCode = originalExitCode; + getMock.mockImplementation(async (path: string) => { + if (path === "/groups/hierarchies") { + return [ + { groupId: 1, parents: [] }, + { groupId: 2, parents: [1] }, + ]; + } + return { name: path }; + }); + } + }); +}); diff --git a/tests/destroy.test.ts b/tests/destroy.test.ts index 2cc4518..bb59718 100644 --- a/tests/destroy.test.ts +++ b/tests/destroy.test.ts @@ -38,6 +38,29 @@ describe("orderDestroy", () => { }; expect(orderDestroy(state, ["mainz", "team"])).toEqual(["team", "mainz"]); }); + + it("orders a child before its parent within the group tier (live hierarchy edges)", () => { + const state = stateWith( + { key: "area", type: "group", id: 1 }, + { key: "kids", type: "group", id: 2 }, + ); + // kids → parent area. State carries no edges, so the command supplies them from /groups/hierarchies. + const edges = new Map([["kids", ["area"]]]); + // Input order deliberately parent-first: a tier-only sort would delete area before kids. + expect(orderDestroy(state, ["area", "kids"], edges)).toEqual(["kids", "area"]); + // Order-independent: the child still precedes the parent regardless of input order. + expect(orderDestroy(state, ["kids", "area"], edges)).toEqual(["kids", "area"]); + }); + + it("still puts a campus (base tier) last, after its child groups", () => { + const state = stateWith( + { key: "mainz", type: "campus", id: 0 }, + { key: "area", type: "group", id: 1 }, + { key: "kids", type: "group", id: 2 }, + ); + const edges = new Map([["kids", ["area"]]]); + expect(orderDestroy(state, ["mainz", "area", "kids"], edges)).toEqual(["kids", "area", "mainz"]); + }); }); describe("runDeleteLoop", () => { diff --git a/tests/execute.test.ts b/tests/execute.test.ts index 122cb5e..21babff 100644 --- a/tests/execute.test.ts +++ b/tests/execute.test.ts @@ -254,6 +254,59 @@ describe("executePlan", () => { expect(calls).toContainEqual({ method: "PUT", path: "/groups/2/parents/1", body: undefined }); }); + it("mirrors config preventDestroy onto the state entry on create (#17 item 2)", async () => { + const state = emptyState("h"); + const { client } = recorder({ "POST /campuses": { id: 5 } }); + const plan: Plan = { + items: [ + { + type: "campus", + key: "zurich", + id: null, + action: "create", + changes: [{ field: "name", from: undefined, to: "Zürich" }], + preventDestroy: true, + }, + ], + }; + await executePlan(plan, { client, state, statePath: "s.json", save: noSave, now: fixedNow }); + expect(state.resources.zurich!.preventDestroy).toBe(true); + }); + + it("persists a preventDestroy toggle even when nothing else changed (no-op) (#17 item 2)", async () => { + const state = emptyState("h"); + state.resources.mainz = { + type: "campus", + id: 0, + key: "mainz", + fields: { name: "Mainz" }, + adoptedAt: "t", + updatedAt: "t", + }; + const { client } = recorder(); + let saved = 0; + const save = async () => { + saved++; + }; + // A note-less no-op whose config now sets preventDestroy — the flag alone is never a diffed field, + // so this is the only chance to persist it to state. + const plan: Plan = { + items: [ + { type: "campus", key: "mainz", id: 0, action: "no-op", changes: [], preventDestroy: true }, + ], + }; + await executePlan(plan, { client, state, statePath: "s.json", save, now: fixedNow }); + expect(state.resources.mainz!.preventDestroy).toBe(true); + expect(saved).toBe(1); // saved exactly once, because the flag actually changed + + // Dropping the flag from config clears it (config → state is the source of truth for protection). + const clearPlan: Plan = { + items: [{ type: "campus", key: "mainz", id: 0, action: "no-op", changes: [] }], + }; + await executePlan(clearPlan, { client, state, statePath: "s.json", save, now: fixedNow }); + expect(state.resources.mainz!.preventDestroy).toBeUndefined(); + }); + it("skips deletes (apply never deletes)", async () => { const state = emptyState("h"); state.resources.old = { diff --git a/tests/resolve.test.ts b/tests/resolve.test.ts new file mode 100644 index 0000000..24b0f85 --- /dev/null +++ b/tests/resolve.test.ts @@ -0,0 +1,18 @@ +import { describe, it, expect } from "vitest"; +import { resolveWithEnv } from "../src/util/resolve.js"; + +describe("resolveWithEnv", () => { + it("prefers the explicit value, trimmed", () => { + expect(resolveWithEnv(" ./x.ts ", "env.ts", "default.ts")).toBe("./x.ts"); + }); + + it("falls back to the env value when explicit is missing or blank", () => { + expect(resolveWithEnv(undefined, "env.ts", "default.ts")).toBe("env.ts"); + expect(resolveWithEnv(" ", "env.ts", "default.ts")).toBe("env.ts"); + }); + + it("falls back to the default when both explicit and env are blank/absent", () => { + expect(resolveWithEnv(undefined, undefined, "default.ts")).toBe("default.ts"); + expect(resolveWithEnv("", " ", "default.ts")).toBe("default.ts"); + }); +}); diff --git a/tests/state.test.ts b/tests/state.test.ts index 6578bca..1be8010 100644 --- a/tests/state.test.ts +++ b/tests/state.test.ts @@ -98,4 +98,47 @@ describe("state.loadState", () => { await writeFile(statePath, JSON.stringify({ version: 2, host: HOST, resources: {} }), "utf8"); await expect(loadState(statePath, HOST)).rejects.toThrow(/Unsupported state file version/); }); + + it("migrates a pre-rename campus snapshot: shortName → shorty (#17 item 4)", async () => { + // A campus adopted before the shortName→shorty rename (Phase 4, no version bump). + const file = { + version: 1, + host: HOST, + resources: { + mainz: { + type: "campus", + id: 0, + key: "mainz", + fields: { name: "Mainz", shortName: "MZ" }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; + await writeFile(statePath, JSON.stringify(file), "utf8"); + const state = await loadState(statePath, HOST); + // The vestigial `shortName` is renamed to the real create-key `shorty`, clearing the phantom drift. + expect(state.resources.mainz!.fields).toEqual({ name: "Mainz", shorty: "MZ" }); + }); + + it("does not clobber a post-rename snapshot that already has shorty", async () => { + const file = { + version: 1, + host: HOST, + resources: { + mainz: { + type: "campus", + id: 0, + key: "mainz", + // Both keys present (e.g. a raw CT snapshot) — the real `shorty` must win, untouched. + fields: { name: "Mainz", shorty: "MZ", shortName: null }, + adoptedAt: "t", + updatedAt: "t", + }, + }, + }; + await writeFile(statePath, JSON.stringify(file), "utf8"); + const state = await loadState(statePath, HOST); + expect(state.resources.mainz!.fields.shorty).toBe("MZ"); + }); });