fix(admin): echo _rev on editor save to prevent silent draft overwrite (#2121)

.changeset/draft-overwrite-rev-2121.md

@@ -0,0 +1,5 @@
+---
+"@emdash-cms/admin": patch
+---
+
+Fixes a silent draft-overwrite in the page editor. The editor now echoes the entry's `_rev` token on save and autosave, so the server rejects a save that is based on a stale read with a 409 conflict instead of silently replacing a newer draft revision. Editors who hit a conflict now see a clear error and can reload instead of losing work.

packages/admin/src/lib/api/content.ts

@@ -59,6 +59,12 @@ export interface ContentItem {
 	liveRevisionId: string | null;
 	draftRevisionId: string | null;
 	seo?: ContentSeo;
+	/**
+	 * Opaque optimistic-concurrency token returned by the content API on
+	 * reads. Echo it back on writes so the server can reject a save that is
+	 * based on a stale read (#2121). Undefined if the server didn't send one.
+	 */
+	_rev?: string;
 }
 
 export interface CreateContentInput {
@@ -114,6 +120,13 @@ export interface UpdateContentInput {
 	/** Skip revision creation (used by autosave) */
 	skipRevision?: boolean;
 	seo?: ContentSeoInput;
+	/**
+	 * Optimistic-concurrency token from the last read. When present, the
+	 * server rejects the write with 409 if the entry changed since that read,
+	 * preventing a stale editor from silently overwriting a newer draft
+	 * (#2121). Omit for a blind write (backwards-compatible).
+	 */
+	_rev?: string;
 }
 
 /**
@@ -238,8 +251,13 @@ export async function fetchContent(
 	if (options?.locale) params.set("locale", options.locale);
 	const query = params.toString() ? `?${params}` : "";
 	const response = await apiFetch(`${API_BASE}/content/${collection}/${id}${query}`);
-	const data = await parseApiResponse<{ item: ContentItem }>(response, "Failed to fetch content");
-	return data.item;
+	const data = await parseApiResponse<{ item: ContentItem; _rev?: string }>(
+		response,
+		"Failed to fetch content",
+	);
+	// The server returns `_rev` at the envelope level, not inside `item`.
+	// Lift it onto the item so the editor can echo it back on save (#2121).
+	return { ...data.item, _rev: data._rev };
 }
 
 /**
@@ -261,8 +279,11 @@ export async function createContent(
 			translationOf: input.translationOf,
 		}),
 	});
-	const data = await parseApiResponse<{ item: ContentItem }>(response, "Failed to create content");
-	return data.item;
+	const data = await parseApiResponse<{ item: ContentItem; _rev?: string }>(
+		response,
+		"Failed to create content",
+	);
+	return { ...data.item, _rev: data._rev };
 }
 
 /**
@@ -282,8 +303,11 @@ export async function updateContent(
 		headers: { "Content-Type": "application/json" },
 		body: JSON.stringify(input),
 	});
-	const data = await parseApiResponse<{ item: ContentItem }>(response, "Failed to update content");
-	return data.item;
+	const data = await parseApiResponse<{ item: ContentItem; _rev?: string }>(
+		response,
+		"Failed to update content",
+	);
+	return { ...data.item, _rev: data._rev };
 }
 
 /**

packages/admin/src/router.tsx

@@ -161,6 +161,8 @@ interface ContentUpdateChanges {
 	bylines?: BylineCreditInput[];
 	skipRevision?: boolean;
 	seo?: ContentSeoInput;
+	/** Echo of the last-read token so the server rejects stale saves (#2121). */
+	_rev?: string;
 }
 
 interface ContentUpdateMutationInput {
@@ -173,7 +175,7 @@ interface ContentUpdateMutationInput {
 interface AutosaveMutationInput {
 	targetId: string;
 	targetLocale?: string;
-	changes: Pick<ContentUpdateChanges, "data" | "slug" | "bylines">;
+	changes: Pick<ContentUpdateChanges, "data" | "slug" | "bylines" | "_rev">;
 }
 
 function patchAutosaveQueries(
@@ -1239,38 +1241,42 @@ function ContentEditPage() {
 	// ContentSettingsPanel, so fresh arrows on every mutation-state flip
 	// (twice per autosave cycle) would defeat the memo. mutate/mutateAsync
 	// are referentially stable.
+	// Echo the optimistic-concurrency token from the last read so the server
+	// rejects a save based on a stale entry (#2121) instead of silently
+	// overwriting a newer draft revision. `rawItem` is the unhydrated content
+	// GET response, which carries `_rev`.
 	const handleSave = React.useCallback(
 		(payload: { data: Record<string, unknown>; slug?: string; bylines?: BylineCreditInput[] }) => {
 			updateMutation.mutate({
 				targetId: id,
 				targetLocale: rawItem?.locale ?? activeLocale,
 				source: "editor",
-				changes: payload,
+				changes: { ...payload, _rev: rawItem?._rev },
 			});
 		},
-		[activeLocale, id, rawItem?.locale, updateMutation.mutate],
+		[activeLocale, id, rawItem?.locale, rawItem?._rev, updateMutation.mutate],
 	);
 
 	const handleAutosave = React.useCallback(
 		(payload: { data: Record<string, unknown>; slug?: string; bylines?: BylineCreditInput[] }) => {
 			autosaveMutation.mutate({
 				targetId: id,
 				targetLocale: rawItem?.locale ?? activeLocale,
-				changes: payload,
+				changes: { ...payload, _rev: rawItem?._rev },
 			});
 		},
-		[activeLocale, autosaveMutation.mutate, id, rawItem?.locale],
+		[activeLocale, autosaveMutation.mutate, id, rawItem?.locale, rawItem?._rev],
 	);
 	const handleAuthorChange = React.useCallback(
 		(authorId: string | null) => {
 			updateMutation.mutate({
 				targetId: id,
 				targetLocale: rawItem?.locale ?? activeLocale,
 				source: "auxiliary",
-				changes: { authorId },
+				changes: { authorId, _rev: rawItem?._rev },
 			});
 		},
-		[activeLocale, id, rawItem?.locale, updateMutation.mutate],
+		[activeLocale, id, rawItem?.locale, rawItem?._rev, updateMutation.mutate],
 	);
 	const handlePublishedAtChange = React.useCallback(
 		(publishedAt: string) => {
@@ -1285,10 +1291,10 @@ function ContentEditPage() {
 				targetId: id,
 				targetLocale: rawItem?.locale ?? activeLocale,
 				source: "auxiliary",
-				changes: { seo },
+				changes: { seo, _rev: rawItem?._rev },
 			});
 		},
-		[activeLocale, id, rawItem?.locale, updateMutation.mutate],
+		[activeLocale, id, rawItem?.locale, rawItem?._rev, updateMutation.mutate],
 	);
 
 	const handlePublish = React.useCallback(() => publishMutation.mutate(), [publishMutation.mutate]);

packages/admin/tests/lib/content-rev.test.ts

@@ -0,0 +1,69 @@
+/**
+ * Regression test for #2121: the admin content API must round-trip `_rev`.
+ *
+ * The content API returns `_rev` on reads and honours it on writes (409 on a
+ * stale token). Before the fix, `ContentItem`/`UpdateContentInput` had no
+ * `_rev` field, so the admin dropped the token on read and never sent it on
+ * save — every editor PUT was a blind write that could silently overwrite a
+ * newer draft revision.
+ */
+import { describe, it, expect, vi, afterEach } from "vitest";
+
+import { fetchContent, updateContent } from "../../src/lib/api/content";
+
+const originalFetch = globalThis.fetch;
+
+afterEach(() => {
+	globalThis.fetch = originalFetch;
+});
+
+function jsonResponse(body: unknown, status = 200) {
+	return new Response(JSON.stringify(body), {
+		status,
+		headers: { "Content-Type": "application/json" },
+	});
+}
+
+describe("content _rev round-trip (#2121)", () => {
+	it("fetchContent surfaces the server-provided _rev on the item", async () => {
+		// The server returns `_rev` at the envelope level (`{ item, _rev }`),
+		// NOT inside `item`. The client must attach it to the returned item so
+		// the editor can echo it back on save. If the client stops doing that,
+		// this test fails — the mock only provides the token at the top level.
+		const item = { id: "01ABC", type: "pages", data: { title: "Hi" } };
+		globalThis.fetch = vi
+			.fn()
+			.mockResolvedValue(jsonResponse({ success: true, data: { item, _rev: "djE6dDE=" } }));
+
+		const fetched = await fetchContent("pages", "01ABC");
+		expect(fetched._rev).toBe("djE6dDE=");
+	});
+
+	it("updateContent sends _rev in the request body", async () => {
+		const fetchSpy = vi
+			.fn()
+			.mockResolvedValue(
+				jsonResponse({ success: true, data: { item: { id: "01ABC" }, _rev: "djI6dDI=" } }),
+			);
+		globalThis.fetch = fetchSpy;
+
+		await updateContent("pages", "01ABC", { data: { title: "New" }, _rev: "djE6dDE=" });
+
+		const [, init] = fetchSpy.mock.calls[0]!;
+		const sent = JSON.parse(init.body as string);
+		expect(sent._rev).toBe("djE6dDE=");
+	});
+
+	it("omits _rev for a blind write when none was read", async () => {
+		const fetchSpy = vi
+			.fn()
+			.mockResolvedValue(jsonResponse({ success: true, data: { item: { id: "01ABC" } } }));
+		globalThis.fetch = fetchSpy;
+
+		await updateContent("pages", "01ABC", { data: { title: "New" } });
+
+		const [, init] = fetchSpy.mock.calls[0]!;
+		const sent = JSON.parse(init.body as string);
+		expect("_rev" in sent).toBe(false);
+	});
+});

packages/admin/tests/router.test.tsx

@@ -47,6 +47,7 @@ vi.mock("../src/components/ContentEditor", () => ({
 		item,
 		onSave,
 		onAutosave,
+		onAuthorChange,
 		onSeoChange,
 		onPublishedAtChange,
 		isSaving,
@@ -59,6 +60,7 @@ vi.mock("../src/components/ContentEditor", () => ({
 		item?: { data?: { title?: string }; slug?: string | null };
 		onSave?: (payload: { data: Record<string, unknown> }) => void;
 		onAutosave?: (payload: { data: Record<string, unknown>; slug?: string }) => void;
+		onAuthorChange?: (authorId: string | null) => void;
 		onSeoChange?: (seo: { title: string }) => void;
 		onPublishedAtChange?: (publishedAt: string) => void;
 		isSaving?: boolean;
@@ -101,6 +103,9 @@ vi.mock("../src/components/ContentEditor", () => ({
 			<button type="button" onClick={() => onSeoChange?.({ title: "Search title" })}>
 				Trigger SEO Sync
 			</button>
+			<button type="button" onClick={() => onAuthorChange?.("user_02")}>
+				Trigger Author Sync
+			</button>
 			<button
 				type="button"
 				disabled={isUpdatingPublishedAt}
@@ -1267,6 +1272,7 @@ describe("ContentEditPage – autosave cache patching", () => {
 			.on("GET", "/_emdash/api/bylines", { data: { items: [] } })
 			.on("GET", "/_emdash/api/content/posts/post_1", {
 				data: {
+					_rev: "revision-token",
 					item: {
 						id: "post_1",
 						type: "posts",
@@ -1369,6 +1375,60 @@ describe("ContentEditPage – autosave cache patching", () => {
 		});
 	});
 
+	it("echoes the current revision token in editor and auxiliary writes", async () => {
+		const { router, TestApp } = buildRouter();
+
+		await router.navigate({
+			to: "/content/$collection/$id",
+			params: { collection: "posts", id: "post_1" },
+		});
+
+		const screen = await render(<TestApp />);
+		await waitFor(() => {
+			expect(screen.getByTestId("mock-title").element().textContent).toBe("Draft Title");
+		});
+
+		const fetchWithMocks = globalThis.fetch;
+		const putBodies: Record<string, unknown>[] = [];
+		globalThis.fetch = (async (input: string | URL | Request, init?: RequestInit) => {
+			const url =
+				typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;
+			if (init?.method === "PUT" && url.includes("/content/posts/post_1")) {
+				if (typeof init.body !== "string") throw new TypeError("Expected a JSON request body");
+				putBodies.push(JSON.parse(init.body) as Record<string, unknown>);
+			}
+			return fetchWithMocks(input, init);
+		}) as typeof fetch;
+
+		try {
+			await screen.getByRole("button", { name: "Save", exact: true }).click();
+			await waitFor(() => expect(putBodies).toHaveLength(1));
+
+			await screen.getByRole("button", { name: "Trigger Draft Sync" }).click();
+			await waitFor(() => expect(putBodies).toHaveLength(2));
+
+			await screen.getByRole("button", { name: "Trigger Author Sync" }).click();
+			await waitFor(() => expect(putBodies).toHaveLength(3));
+
+			await screen.getByRole("button", { name: "Trigger SEO Sync" }).click();
+			await waitFor(() => expect(putBodies).toHaveLength(4));
+
+			expect(putBodies).toEqual([
+				{ data: { title: "Test Post" }, _rev: "revision-token" },
+				{
+					data: { title: "Autosaved Title" },
+					slug: "autosaved-title",
+					_rev: "revision-token",
+					skipRevision: true,
+				},
+				{ authorId: "user_02", _rev: "revision-token" },
+				{ seo: { title: "Search title" }, _rev: "revision-token" },
+			]);
+		} finally {
+			globalThis.fetch = fetchWithMocks;
+		}
+	});
+
 	it("sends publish-date changes through the auxiliary update payload", async () => {
 		const fetchWithMocks = globalThis.fetch;
 		let updateBody: Record<string, unknown> | undefined;