fix(core): let an entry shed a deleted field's value instead of stranding it

.changeset/deleted-field-stranded-drafts.md

@@ -0,0 +1,5 @@
+---
+"emdash": patch
+---
+
+Fixes an entry with a pending draft becoming unsaveable after one of its fields is deleted. The draft revision stores the whole `data`, so the deleted field's value stayed in it and every read handed it back; writing that data back was then refused with `unknown field on collection`, an explicit `null` was refused too, and omitting the key was accepted but left the merge carrying it, so no request body got the entry out of the state. An update now drops a key the collection has no field for when the entry already stores it, and a saved entry is written without those keys, so it sheds them. A key the entry does not already store is still reported as an unknown field. This applies to every content write, including those made from a plugin or a sandboxed plugin bridge.

packages/core/src/content/known-fields.ts

@@ -0,0 +1,44 @@
+/**
+ * Reconcile an entry's keys with the fields its collection currently declares.
+ *
+ * A draft revision stores the whole `data` as JSON, so deleting a field leaves the old
+ * value inside the revision. Every content-write surface needs the same two rules to keep
+ * that from stranding the entry, so they live here rather than in one caller.
+ */
+
+/** Keys EmDash reserves (`_slug` and the like). No field declares them. */
+function isReservedKey(key: string): boolean {
+	return key.startsWith("_");
+}
+
+/**
+ * The entry data without the keys the collection has no field for.
+ *
+ * Reserved keys are kept.
+ */
+export function keepKnownFields(
+	data: Record<string, unknown>,
+	knownFieldSlugs: ReadonlySet<string>,
+): Record<string, unknown> {
+	const result: Record<string, unknown> = {};
+	for (const [key, value] of Object.entries(data)) {
+		if (isReservedKey(key) || knownFieldSlugs.has(key)) result[key] = value;
+	}
+	return result;
+}
+
+/**
+ * Incoming keys with no matching field that the entry already stores.
+ *
+ * Dropping these lets a read-then-write save succeed after the field was deleted. A key the
+ * entry has never stored is not returned, so a genuinely unknown field is still reported.
+ */
+export function staleStoredKeys(
+	data: Record<string, unknown>,
+	stored: Record<string, unknown>,
+	knownFieldSlugs: ReadonlySet<string>,
+): string[] {
+	return Object.keys(data).filter(
+		(key) => !isReservedKey(key) && !knownFieldSlugs.has(key) && Object.hasOwn(stored, key),
+	);
+}

packages/core/src/database/repositories/content.ts

@@ -2,6 +2,7 @@ import { sql, type Kysely } from "kysely";
 import { ulid } from "ulidx";
 
 import type { ContentFieldFilterValue, ContentFieldFilters } from "../../content-list-query.js";
+import { keepKnownFields, staleStoredKeys } from "../../content/known-fields.js";
 import { normalizeExplicitDatetime } from "../../datetime-normalization.js";
 import { invalidateCollectionCache } from "../../object-cache/index.js";
 import { isIndexableFieldType, type FieldType } from "../../schema/types.js";
@@ -963,24 +964,44 @@ export class ContentRepository {
 			return this.update(type, id, { ...input, data });
 		}
 
-		const fieldSlugs = new Set(collectionRows.map((row) => row.fieldSlug).filter(Boolean));
-		for (const field of Object.keys(data)) {
+		const fieldSlugs = new Set(
+			collectionRows.map((row) => row.fieldSlug).filter((slug): slug is string => Boolean(slug)),
+		);
+
+		const revisionRepo = new RevisionRepository(this.db);
+		let existing = await this.findById(type, id);
+
+		// A key with no field is rejected unless the entry already stores it: deleting a field
+		// leaves its value in the draft revision, which holds the whole `data`, so a caller that
+		// reads an entry and writes it back would otherwise be refused for a key it never chose
+		// to send. Those keys are dropped here and shed from the merge below.
+		let incoming = data;
+		if (existing) {
+			let stored: Record<string, unknown> = existing.data ?? {};
+			if (existing.draftRevisionId) {
+				const draft = await revisionRepo.findById(existing.draftRevisionId);
+				if (draft?.data) stored = draft.data;
+			}
+			const stale = staleStoredKeys(data, stored, fieldSlugs);
+			if (stale.length > 0) {
+				incoming = { ...data };
+				for (const key of stale) delete incoming[key];
+			}
+		}
+		for (const field of Object.keys(incoming)) {
 			if (!fieldSlugs.has(field)) {
 				throw new EmDashValidationError(`Unknown field '${field}' in collection '${type}'`);
 			}
 		}
 
-		const revisionRepo = new RevisionRepository(this.db);
-		let existing = await this.findById(type, id);
-
 		for (let attempt = 0; existing && attempt < MAX_DRAFT_STAGE_ATTEMPTS; attempt++) {
 			let baseData = existing.data;
 			if (existing.draftRevisionId) {
 				const draft = await revisionRepo.findById(existing.draftRevisionId);
 				if (draft) baseData = draft.data;
 			}
 
-			const mergedData = { ...baseData, ...data };
+			const mergedData = keepKnownFields({ ...baseData, ...incoming }, fieldSlugs);
 			if (stagedSlug !== undefined) mergedData._slug = stagedSlug;
 			const revision = await revisionRepo.create({
 				collection: type,

packages/core/src/emdash-runtime.ts

@@ -30,6 +30,7 @@ import type { EmDashManifest } from "./astro/types.js";
 import { getAuthMode } from "./auth/mode.js";
 import { getTrustedProxyHeaders } from "./auth/trusted-proxy.js";
 import type { ContentFieldFilters } from "./content-list-query.js";
+import { keepKnownFields, staleStoredKeys } from "./content/known-fields.js";
 import { isSqlite } from "./database/dialect-helpers.js";
 import { kyselyLogOption } from "./database/instrumentation.js";
 import {
@@ -94,6 +95,7 @@ import type {
 } from "./plugins/types.js";
 import { recordSchedulerHeartbeatSafely } from "./scheduler-health.js";
 import { primeRegisteredCollections } from "./schema/collection-slugs-cache.js";
+import type { CollectionWithFields } from "./schema/types.js";
 import { isMissingTableError } from "./utils/db-errors.js";
 import { hashString } from "./utils/hash.js";
 import { createInitLock, type InitLock, initWithLock } from "./utils/init-lock.js";
@@ -3054,6 +3056,13 @@ export class EmDashRuntime {
 		}
 		const { _rev: _discardedRev, actor: _discardedActor, ...bodyWithoutRev } = body;
 
+		// Loaded once and threaded through normalization, the stale-key drop and the draft
+		// merge below: each of those needs the field list and the registry does not cache.
+		const collectionInfo = bodyWithoutRev.data
+			? await this.schemaRegistry.getCollectionWithFields(collection).catch(() => null)
+			: null;
+		const knownFieldSlugs = new Set((collectionInfo?.fields ?? []).map((f) => f.slug));
+
 		// Run beforeSave hooks if data is provided
 		let processedData = bodyWithoutRev.data;
 		if (bodyWithoutRev.data) {
@@ -3073,7 +3082,18 @@ export class EmDashRuntime {
 			}
 
 			// Normalize media fields (fill dimensions, storageKey, etc.)
-			processedData = await this.normalizeMediaFields(collection, processedData!);
+			processedData = await this.normalizeMediaFields(collection, processedData!, collectionInfo);
+
+			// Drop unknown field keys the entry already stores (e.g. a deleted field
+			// stranded in a draft revision) before validation, while still rejecting
+			// unknown keys the entry has never stored.
+			if (collectionInfo?.fields) {
+				processedData = await this.dropUnknownKeysAlreadyStored(
+					processedData,
+					resolvedItem,
+					knownFieldSlugs,
+				);
+			}
 
 			// Validate field-level shape BEFORE the draft-revision write so
 			// invalid updates can't silently land in revision history.
@@ -3095,7 +3115,6 @@ export class EmDashRuntime {
 		let usesDraftRevisions = false;
 		let draftStorageChanged = false;
 		if (processedData) {
-			const collectionInfo = await this.schemaRegistry.getCollectionWithFields(collection);
 			if (collectionInfo?.supports?.includes("revisions")) {
 				usesDraftRevisions = true;
 				const revisionRepo = new RevisionRepository(this.db);
@@ -3110,7 +3129,12 @@ export class EmDashRuntime {
 						baseData = existing.data;
 					}
 
-					const mergedData = { ...baseData, ...processedData };
+					// Written without the keys the collection has no field for, so an entry
+					// carrying a deleted field's value sheds it on its next save instead of
+					// carrying it through every revision that follows.
+					const mergedData = collectionInfo?.fields
+						? keepKnownFields({ ...baseData, ...processedData }, knownFieldSlugs)
+						: { ...baseData, ...processedData };
 					if (bodyWithoutRev.slug !== undefined) {
 						mergedData._slug = bodyWithoutRev.slug;
 					}
@@ -4152,19 +4176,53 @@ export class EmDashRuntime {
 		return undefined;
 	}
 
+	/**
+	 * Drop incoming keys that have no matching collection field when the entry
+	 * already stores them in its live data or current draft revision.
+	 *
+	 * Draft revisions keep the full `data` JSON, so deleting a field can strand
+	 * the old value; this lets a read-then-write save succeed without allowing
+	 * genuinely unknown keys.
+	 */
+	private async dropUnknownKeysAlreadyStored(
+		data: Record<string, unknown>,
+		existing: { data: Record<string, unknown>; draftRevisionId?: string | null } | null,
+		knownFieldSlugs: ReadonlySet<string>,
+	): Promise<Record<string, unknown>> {
+		if (!existing) return data;
+
+		let stored: Record<string, unknown> = existing.data ?? {};
+		if (existing.draftRevisionId) {
+			const draft = await new RevisionRepository(this.db)
+				.findById(existing.draftRevisionId)
+				.catch(() => null);
+			if (draft?.data) stored = draft.data;
+		}
+
+		const stale = staleStoredKeys(data, stored, knownFieldSlugs);
+		if (stale.length === 0) return data;
+
+		const result = { ...data };
+		for (const key of stale) delete result[key];
+		return result;
+	}
+
 	/**
 	 * Normalize image/file fields in content data.
 	 * Fills missing dimensions, storageKey, mimeType, and filename from providers.
 	 */
 	private async normalizeMediaFields(
 		collection: string,
 		data: Record<string, unknown>,
+		preloaded?: CollectionWithFields | null,
 	): Promise<Record<string, unknown>> {
-		let collectionInfo;
-		try {
-			collectionInfo = await this.schemaRegistry.getCollectionWithFields(collection);
-		} catch {
-			return data;
+		let collectionInfo = preloaded;
+		if (collectionInfo === undefined) {
+			try {
+				collectionInfo = await this.schemaRegistry.getCollectionWithFields(collection);
+			} catch {
+				return data;
+			}
 		}
 		if (!collectionInfo?.fields) return data;
 

packages/core/tests/integration/content/deleted-field-stranded-entry.test.ts

@@ -0,0 +1,126 @@
+/**
+ * Deleting a field can strand a pending draft: the revision stores the whole `data`,
+ * so the deleted field's value stays there and every read hands it back. Writing that
+ * data back was refused as an unknown field, an explicit null was refused too, and
+ * omitting the key was accepted but left the merge carrying it, so no request body
+ * got the entry out of the state.
+ */
+import { afterEach, beforeEach, describe, expect } from "vitest";
+
+import { ContentRepository } from "../../../src/database/repositories/content.js";
+import type { EmDashRuntime } from "../../../src/emdash-runtime.js";
+import { SchemaRegistry } from "../../../src/schema/registry.js";
+import { createTestRuntime } from "../../utils/mcp-runtime.js";
+import {
+	describeEachDialect,
+	setupForDialect,
+	teardownForDialect,
+	type DialectTestContext,
+} from "../../utils/test-db.js";
+
+describeEachDialect(
+	"an entry with a pending draft when one of its fields is deleted",
+	(dialect) => {
+		let ctx: DialectTestContext;
+		let runtime: EmDashRuntime;
+		let id: string;
+
+		beforeEach(async () => {
+			ctx = await setupForDialect(dialect);
+			const registry = new SchemaRegistry(ctx.db);
+			await registry.createCollection({ slug: "posts", label: "Posts" });
+			await registry.createField("posts", { slug: "title", label: "Title", type: "string" });
+			await registry.createField("posts", { slug: "hits", label: "Hits", type: "integer" });
+			runtime = createTestRuntime(ctx.db);
+
+			const created = await runtime.handleContentCreate("posts", {
+				slug: "p1",
+				data: { title: "p1", hits: 42 },
+				status: "published",
+			});
+			if (!created.success) throw new Error("setup: create failed");
+			id = created.data.item.id;
+
+			// A pending draft, as an editor leaves behind by saving without publishing.
+			const draft = await runtime.handleContentUpdate("posts", id, {
+				data: { title: "p1 draft", hits: 43 },
+			});
+			if (!draft.success) throw new Error("setup: draft update failed");
+
+			await registry.deleteField("posts", "hits");
+		});
+
+		afterEach(async () => {
+			await teardownForDialect(ctx);
+		});
+
+		it("saves the data it just read, and sheds the deleted field", async () => {
+			const got = await runtime.handleContentGet("posts", id);
+			expect(got.success).toBe(true);
+			if (!got.success) return;
+			const data = { ...(got.data.item.data as Record<string, unknown>), title: "edited" };
+			expect(data.hits).toBe(43); // the draft still carries it, which is what a client sends back
+
+			const saved = await runtime.handleContentUpdate("posts", id, { data });
+			expect(saved.success).toBe(true);
+			if (!saved.success) return;
+			expect(saved.data.item.data).toMatchObject({ title: "edited" });
+			expect(Object.hasOwn(saved.data.item.data as Record<string, unknown>, "hits")).toBe(false);
+		});
+
+		it("accepts an explicit null for the deleted field, and refuses it once the entry is clean", async () => {
+			const cleared = await runtime.handleContentUpdate("posts", id, {
+				data: { title: "edited", hits: null },
+			});
+			expect(cleared.success).toBe(true);
+
+			// The entry no longer stores the key, so sending it again is an unknown field like any other.
+			const again = await runtime.handleContentUpdate("posts", id, {
+				data: { title: "edited twice", hits: null },
+			});
+			expect(again.success).toBe(false);
+		});
+
+		it("drops the value from the merged data even when the key is omitted", async () => {
+			const saved = await runtime.handleContentUpdate("posts", id, { data: { title: "edited" } });
+			expect(saved.success).toBe(true);
+			if (!saved.success) return;
+			expect(Object.hasOwn(saved.data.item.data as Record<string, unknown>, "hits")).toBe(false);
+		});
+
+		it("still refuses a key the entry does not already store", async () => {
+			const saved = await runtime.handleContentUpdate("posts", id, {
+				data: { title: "edited", titel: "typo" },
+			});
+			expect(saved.success).toBe(false);
+			if (saved.success) return;
+			expect(saved.error.message).toContain("unknown field on collection");
+		});
+
+		// The plugin context and both sandbox bridges write through the repository rather than
+		// the runtime handler, so the same entry has to be saveable from there too.
+		describe("through ContentRepository.updateDraftAware", () => {
+			it("saves the data it just read, and sheds the deleted field", async () => {
+				const repo = new ContentRepository(ctx.db);
+				const got = await runtime.handleContentGet("posts", id);
+				expect(got.success).toBe(true);
+				if (!got.success) return;
+				const read = got.data.item.data as Record<string, unknown>;
+				expect(read.hits).toBe(43); // the draft still carries it, which is what a caller sends back
+
+				const updated = await repo.updateDraftAware("posts", id, {
+					data: { ...read, title: "edited" },
+				});
+				expect(updated.data).toMatchObject({ title: "edited" });
+				expect(Object.hasOwn(updated.data, "hits")).toBe(false);
+			});
+
+			it("still refuses a key the entry does not already store", async () => {
+				const repo = new ContentRepository(ctx.db);
+				await expect(
+					repo.updateDraftAware("posts", id, { data: { title: "edited", titel: "typo" } }),
+				).rejects.toThrow(/Unknown field/);
+			});
+		});
+	},
+);