fix: pull writes canonical model files and detects changes reliably

src/commands/pull.ts

@@ -88,17 +88,15 @@ export default createCommand(config, async ({ values }) => {
 		localCustomTypes.map((customType) => customType.model),
 		{
 			getKey: (model) => model.id,
-			equals: (a, b) =>
-				JSON.stringify(canonicalizeCustomType(a)) === JSON.stringify(canonicalizeCustomType(b)),
+			equals: (remote, local) => JSON.stringify(canonicalizeCustomType(remote)) === JSON.stringify(local),
 		},
 	);
 	const sliceOps = diffArrays(
 		remoteSlices,
 		localSlices.map((slice) => slice.model),
 		{
 			getKey: (model) => model.id,
-			equals: (a, b) =>
-				JSON.stringify(canonicalizeSlice(a)) === JSON.stringify(canonicalizeSlice(b)),
+			equals: (remote, local) => JSON.stringify(canonicalizeSlice(remote)) === JSON.stringify(local),
 		},
 	);
 

src/commands/sync.ts

@@ -9,6 +9,7 @@ import { createCommand, type CommandConfig, CommandError } from "../lib/command"
 import { diffArrays } from "../lib/diff";
 import { getCustomTypes, getSlices } from "../lib/prismic/clients/custom-types";
 import { completeOnboardingStepsSilently } from "../lib/prismic/clients/repository";
+import { canonicalizeCustomType, canonicalizeSlice } from "../lib/prismic/models";
 import { getRepositoryName } from "../project";
 import { trackCommandStart, trackCommandEnd } from "../tracking";
 
@@ -83,7 +84,11 @@ export default createCommand(config, async ({ values }) => {
 
 				const changed: string[] = [];
 
-				const sliceOps = diffArrays(remoteSlices, localSliceModels, { getKey: (m) => m.id });
+				const sliceOps = diffArrays(remoteSlices, localSliceModels, {
+					getKey: (m) => m.id,
+					equals: (remote, local) =>
+						JSON.stringify(canonicalizeSlice(remote)) === JSON.stringify(local),
+				});
 				if (sliceOps.insert.length + sliceOps.update.length + sliceOps.delete.length > 0) {
 					for (const slice of sliceOps.update) {
 						await adapter.updateSlice(slice);
@@ -99,6 +104,8 @@ export default createCommand(config, async ({ values }) => {
 
 				const customTypeOps = diffArrays(remoteCustomTypes, localCustomTypeModels, {
 					getKey: (m) => m.id,
+					equals: (remote, local) =>
+						JSON.stringify(canonicalizeCustomType(remote)) === JSON.stringify(local),
 				});
 				if (
 					customTypeOps.insert.length + customTypeOps.update.length + customTypeOps.delete.length >

src/lib/prismic/models.ts

@@ -196,8 +196,8 @@ export function canonicalizeSlice(model: SharedSlice): SharedSlice {
 		...sortKeys(model),
 		variations: model.variations.map((variation) => {
 			const sorted = sortKeys(variation);
-			if (sorted.primary) sorted.primary = canonicalizeFields(sorted.primary);
-			if (sorted.items) sorted.items = canonicalizeFields(sorted.items);
+			if (variation.primary) sorted.primary = canonicalizeFields(variation.primary);
+			if (variation.items) sorted.items = canonicalizeFields(variation.items);
 			return sorted;
 		}),
 	};
@@ -207,19 +207,28 @@ function canonicalizeFields<F extends DynamicWidget>(fields: Record<string, F>):
 	return Object.fromEntries(
 		Object.entries(fields).map(([id, field]) => {
 			const sorted = sortKeys(field);
-			if ("config" in sorted && sorted.config) {
-				sorted.config = sortKeys(sorted.config);
-				const group = sorted.config as { fields?: Fields };
-				if (group.fields) group.fields = canonicalizeFields(group.fields);
+			if (
+				field.type === "Group" &&
+				field.config?.fields &&
+				sorted.type === "Group" &&
+				sorted.config?.fields
+			) {
+				sorted.config.fields = canonicalizeFields(field.config.fields);
 			}
 			return [id, sorted];
 		}),
 	);
 }
 
+// Sorts keys recursively. Entry order of field maps encodes field position,
+// so callers restore those from the unsorted input.
 function sortKeys<T>(object: T): T {
+	if (Array.isArray(object)) return object.map(sortKeys) as T;
+	if (object === null || typeof object !== "object") return object;
 	return Object.fromEntries(
-		Object.entries(object as Record<string, unknown>).sort(([a], [b]) => a.localeCompare(b)),
+		Object.entries(object)
+			.sort(([a], [b]) => a.localeCompare(b))
+			.map(([key, value]) => [key, sortKeys(value)]),
 	) as T;
 }
 

test/it.ts

@@ -218,6 +218,22 @@ export function buildSlice(overrides?: Partial<SharedSlice>): SharedSlice {
 	};
 }
 
+// Reverses object key order at every depth, except field maps (`json` tabs,
+// group `fields`, and slice variation `primary`/`items`), whose entry order is
+// position-significant.
+export function scramble<T>(value: T, keepOrder = 0): T {
+	if (Array.isArray(value)) return value.map((child) => scramble(child)) as T;
+	if (value === null || typeof value !== "object") return value;
+	const entries = Object.entries(value).map(([key, child]) => [
+		key,
+		scramble(
+			child,
+			key === "json" ? 2 : ["fields", "primary", "items"].includes(key) ? 1 : keepOrder - 1,
+		),
+	]);
+	return Object.fromEntries(keepOrder > 0 ? entries : entries.reverse());
+}
+
 export async function writeLocalCustomType(project: URL, model: CustomType): Promise<void> {
 	const path = new URL(`customtypes/${model.id}/index.json`, project);
 	await mkdir(new URL(".", path), { recursive: true });

test/pull.test.ts

@@ -1,10 +1,11 @@
 import { pascalCase } from "change-case";
-import { writeFile, mkdir } from "node:fs/promises";
+import { readFile, writeFile, mkdir } from "node:fs/promises";
 import { sep } from "node:path";
 import { fileURLToPath } from "node:url";
 import { x } from "tinyexec";
+import { describe } from "vitest";
 
-import { buildCustomType, buildSlice, it } from "./it";
+import { buildCustomType, buildSlice, it, scramble } from "./it";
 import {
 	deleteCustomType,
 	deleteSlice,
@@ -250,6 +251,103 @@ it.sequential("removes route when page type is deleted", async ({
 	await expect(project).not.toHaveRoute({ type: customType.id });
 });
 
+describe("with an isolated repository", () => {
+	it.scoped({ isolateRepo: true });
+
+	it("writes canonical model files that later pulls leave untouched", async ({
+		expect,
+		project,
+		prismic,
+		repo,
+		token,
+		host,
+	}) => {
+		// Nested config objects with unsorted keys, plus field maps (tab, group,
+		// slice primary) whose entry order must be kept as-is.
+		const customType = buildCustomType({
+			json: {
+				Main: {
+					social_image: {
+						type: "Image",
+						config: {
+							label: "Social image",
+							constraint: { width: 1200, height: 630 },
+							thumbnails: [{ name: "small", width: 100, height: 50 }],
+						},
+					},
+					links: {
+						type: "Group",
+						config: {
+							label: "Links",
+							fields: {
+								url: { type: "Text", config: { label: "URL", placeholder: "" } },
+								label: { type: "Text", config: { label: "Label", placeholder: "" } },
+							},
+						},
+					},
+				},
+			},
+		} as Partial<ReturnType<typeof buildCustomType>>);
+		const slice = buildSlice();
+		slice.variations[0].primary = {
+			title: { type: "Text", config: { placeholder: "Enter a title", label: "Title" } },
+			subtitle: { type: "Text", config: { placeholder: "Enter a subtitle", label: "Subtitle" } },
+		};
+
+		await Promise.all([
+			insertCustomType(customType, { repo, token, host }),
+			insertSlice(slice, { repo, token, host }),
+		]);
+
+		const first = await prismic("pull", ["--repo", repo]);
+		expect(first.exitCode, first.stderr).toBe(0);
+
+		const typePath = new URL(`customtypes/${customType.id}/index.json`, project);
+		const slicePath = new URL(`slices/${pascalCase(slice.name)}/model.json`, project);
+		const pulledType = await readFile(typePath, "utf8");
+		const pulledSlice = await readFile(slicePath, "utf8");
+
+		// Metadata and config keys are sorted; field order is kept.
+		const writtenType = JSON.parse(pulledType);
+		expect(Object.keys(writtenType.json.Main)).toEqual(["social_image", "links"]);
+		expect(Object.keys(writtenType.json.Main.social_image.config.constraint)).toEqual([
+			"height",
+			"width",
+		]);
+		expect(Object.keys(writtenType.json.Main.links.config.fields)).toEqual(["url", "label"]);
+		const writtenSlice = JSON.parse(pulledSlice);
+		expect(Object.keys(writtenSlice.variations[0].primary)).toEqual(["title", "subtitle"]);
+
+		// A second pull with no changes on either side must not touch the files.
+		const second = await prismic("pull", ["--repo", repo]);
+		expect(second.exitCode, second.stderr).toBe(0);
+		expect(second.stdout).toContain("Already up to date.");
+		expect(await readFile(typePath, "utf8")).toBe(pulledType);
+		expect(await readFile(slicePath, "utf8")).toBe(pulledSlice);
+
+		// Files with non-canonical key order count as updates. This project has
+		// no git repo to protect local edits, so a plain pull refuses; --force
+		// writes the files back in canonical form.
+		const scrambledType = JSON.stringify(scramble(JSON.parse(pulledType)), null, 2);
+		const scrambledSlice = JSON.stringify(scramble(JSON.parse(pulledSlice)), null, 2);
+		expect(scrambledType).not.toBe(pulledType);
+		expect(scrambledSlice).not.toBe(pulledSlice);
+		await writeFile(typePath, scrambledType);
+		await writeFile(slicePath, scrambledSlice);
+
+		const blocked = await prismic("pull", ["--repo", repo]);
+		expect(blocked.exitCode).toBe(1);
+		expect(blocked.stderr).toContain("--force");
+
+		const rewrite = await prismic("pull", ["--repo", repo, "--force"]);
+		expect(rewrite.exitCode, rewrite.stderr).toBe(0);
+		expect(rewrite.stdout).toContain("updated 1, deleted 0 types");
+		expect(rewrite.stdout).toContain("updated 1, deleted 0 slices");
+		expect(await readFile(typePath, "utf8")).toBe(pulledType);
+		expect(await readFile(slicePath, "utf8")).toBe(pulledSlice);
+	});
+});
+
 it.sequential("blocks pull when local model files have uncommitted changes", async ({
 	expect,
 	project,

test/status.test.ts

@@ -1,6 +1,15 @@
 import { describe } from "vitest";
 
-import { buildCustomType, buildSlice, it, readLocalCustomType, writeLocalCustomType } from "./it";
+import {
+	buildCustomType,
+	buildSlice,
+	it,
+	readLocalCustomType,
+	readLocalSlice,
+	scramble,
+	writeLocalCustomType,
+	writeLocalSlice,
+} from "./it";
 import { insertCustomType, insertSlice } from "./prismic";
 
 it("supports --help", async ({ expect, prismic }) => {
@@ -62,52 +71,75 @@ describe("with an isolated repository", () => {
 		expect(stdout).toContain("prismic pull");
 	});
 
-	it("reports in-sync when local only reorders metadata and config keys", async ({
+	it("reports in-sync and push writes nothing when local only reorders keys", async ({
 		expect,
 		project,
 		prismic,
 		repo,
 		token,
 		host,
 	}) => {
-		// A field with multiple config keys, so config key order can be reordered.
 		const customType = buildCustomType({
 			json: {
 				Main: {
 					title: { type: "Text", config: { label: "Title", placeholder: "Enter a title" } },
+					social_image: {
+						type: "Image",
+						config: {
+							label: "Social image",
+							constraint: { width: 1200, height: 630 },
+							thumbnails: [{ name: "small", width: 100, height: 50 }],
+						},
+					},
+					links: {
+						type: "Group",
+						config: {
+							label: "Links",
+							fields: {
+								url: { type: "Text", config: { label: "URL", placeholder: "" } },
+								label: { type: "Text", config: { label: "Label", placeholder: "" } },
+							},
+						},
+					},
 				},
 			},
 		} as Partial<ReturnType<typeof buildCustomType>>);
-		await insertCustomType(customType, { repo, token, host });
+		const slice = buildSlice();
+		slice.variations[0].primary = {
+			title: { type: "Text", config: { placeholder: "Enter a title", label: "Title" } },
+		};
+		await Promise.all([
+			insertCustomType(customType, { repo, token, host }),
+			insertSlice(slice, { repo, token, host }),
+		]);
 
 		// Pull writes the canonical form to disk.
 		const pull = await prismic("pull", ["--repo", repo]);
 		expect(pull.exitCode, pull.stderr).toBe(0);
 
-		// Hand-edit the local file: reverse the order of metadata keys and of each
-		// field's config keys, leaving all values and the field order unchanged.
-		const pulled = await readLocalCustomType(project, customType.id);
-		// Pull writes the canonical (sorted-key) form, not the raw API key order.
-		expect(Object.keys(pulled)).toEqual(Object.keys(pulled).sort());
-		const canonical = JSON.stringify(pulled, null, 2);
-		for (const fields of Object.values(pulled.json)) {
-			for (const field of Object.values(fields)) {
-				const f = field as { config?: Record<string, unknown> };
-				if (f.config) {
-					expect(Object.keys(f.config)).toEqual(Object.keys(f.config).sort());
-					f.config = Object.fromEntries(Object.entries(f.config).reverse());
-				}
-			}
-		}
-		const scrambled = Object.fromEntries(Object.entries(pulled).reverse()) as typeof pulled;
+		// Hand-edit the local files: reverse key order at every depth, leaving all
+		// values and the field order unchanged.
+		const pulledType = await readLocalCustomType(project, customType.id);
+		const scrambledType = scramble(pulledType);
 		// Confirm the hand-edit really produced a non-canonical file.
-		expect(JSON.stringify(scrambled, null, 2)).not.toBe(canonical);
-		await writeLocalCustomType(project, scrambled);
+		expect(JSON.stringify(scrambledType, null, 2)).not.toBe(JSON.stringify(pulledType, null, 2));
+		await writeLocalCustomType(project, scrambledType);
+
+		const pulledSlice = await readLocalSlice(project, slice.id);
+		if (!pulledSlice) throw new Error(`Slice "${slice.id}" was not pulled.`);
+		const scrambledSlice = scramble(pulledSlice);
+		expect(JSON.stringify(scrambledSlice, null, 2)).not.toBe(JSON.stringify(pulledSlice, null, 2));
+		await writeLocalSlice(project, scrambledSlice);
 
 		// Both sides canonicalize equal, so status must report no changes.
 		const { stdout, stderr, exitCode } = await prismic("status", ["--repo", repo]);
 		expect(exitCode, stderr).toBe(0);
 		expect(stdout).toContain("Already up to date.");
+
+		// Push uses the same comparison, so it must not update the remote models.
+		const push = await prismic("push", ["--repo", repo]);
+		expect(push.exitCode, push.stderr).toBe(0);
+		expect(push.stdout).toContain("Already up to date.");
 	});
 
 	it("reports differing models when local and remote disagree", async ({

test/sync.test.ts

@@ -36,5 +36,17 @@ describe("with an isolated repository", () => {
 
 		await expect(project).toContainCustomType(customType);
 		await expect(project).toContainSlice(slice);
+
+		// A later remote change must only re-sync models that actually changed.
+		// The custom type's local file has canonical key order while the remote
+		// returns its own order, so this fails if sync compares raw JSON.
+		const outputLengthBeforeSliceB = output().length;
+		const newOutput = () => output().slice(outputLengthBeforeSliceB);
+		const sliceB = buildSlice();
+		await insertSlice(sliceB, { repo, token, host });
+
+		await expect.poll(newOutput, { timeout: 30_000 }).toContain("Changes detected in slices");
+		expect(newOutput()).not.toContain("custom types");
+		await expect(project).toContainSlice(sliceB);
 	}, 60_000);
 });