diff --git a/.changeset/atomic-expected-publish.md b/.changeset/atomic-expected-publish.md new file mode 100644 index 0000000000..b64e17ef2c --- /dev/null +++ b/.changeset/atomic-expected-publish.md @@ -0,0 +1,5 @@ +--- +"emdash": patch +--- + +Fixes publication workflows so callers can pass the approved `_rev` to publish, unpublish, or discard a draft and receive a `CONFLICT` response when the entry changed. Calls that omit `_rev` keep the existing behavior. diff --git a/packages/core/src/api/handlers/content.ts b/packages/core/src/api/handlers/content.ts index 0b54b6fdb0..9403e42987 100644 --- a/packages/core/src/api/handlers/content.ts +++ b/packages/core/src/api/handlers/content.ts @@ -10,7 +10,11 @@ import { isSqlite } from "../../database/dialect-helpers.js"; import { BylineRepository } from "../../database/repositories/byline.js"; import type { ContentBylineInput } from "../../database/repositories/byline.js"; import { CommentRepository } from "../../database/repositories/comment.js"; -import { ContentRepository, isSystemOrderField } from "../../database/repositories/content.js"; +import { + ContentRepository, + isSystemOrderField, + type ContentRevisionPrecondition, +} from "../../database/repositories/content.js"; import { RedirectRepository } from "../../database/repositories/redirect.js"; import { RevisionRepository } from "../../database/repositories/revision.js"; import { SeoRepository } from "../../database/repositories/seo.js"; @@ -39,7 +43,7 @@ import { invalidateRedirectCache } from "../../redirects/cache.js"; import { FTSManager } from "../../search/fts-manager.js"; import { invalidateTermCache } from "../../taxonomies/index.js"; import { isMissingColumnError, isMissingTableError } from "../../utils/db-errors.js"; -import { encodeRev, validateRev } from "../rev.js"; +import { decodeRev, encodeRev, validateRev } from "../rev.js"; import type { ApiResult, ContentListResponse, ContentResponse } from "../types.js"; import { validateMediaFields } from "./validate-media-fields.js"; @@ -59,6 +63,15 @@ function hasApiError(error: unknown): error is Error & { apiError: { code: strin ); } +function decodeRevisionPrecondition( + rev: string | undefined, +): ContentRevisionPrecondition | undefined { + if (rev === undefined) return undefined; + const decoded = decodeRev(rev); + if (!decoded) throw new ContentMutationConflictError("Revision precondition did not match"); + return decoded; +} + /** * Extract a slug source (title or name) from content data. * Returns null if no suitable string field is found. @@ -1584,9 +1597,11 @@ export async function handleContentPublish( publishedAt?: string; requireScheduledDue?: boolean; expectedScheduledAt?: string; + _rev?: string; } = {}, ): Promise> { try { + const expectedRevision = decodeRevisionPrecondition(options._rev); const item = await withTransaction(db, async (trx) => { const repo = new ContentRepository(trx); const resolvedId = (await resolveId(repo, collection, id)) ?? id; @@ -1607,6 +1622,7 @@ export async function handleContentPublish( options.expectedScheduledAt, publishConfig.supportsRevisions, publishConfig.routable, + expectedRevision, ); // Leave a 301 behind when publishing changed the slug of an entry that @@ -1629,7 +1645,7 @@ export async function handleContentPublish( return { success: true, - data: { item }, + data: { item, _rev: encodeRev(item) }, }; } catch (error) { if (error instanceof ContentMutationConflictError) { @@ -1707,12 +1723,14 @@ export async function handleContentUnpublish( db: Kysely, collection: string, id: string, + options: { _rev?: string } = {}, ): Promise> { try { + const expectedRevision = decodeRevisionPrecondition(options._rev); const item = await withTransaction(db, async (trx) => { const repo = new ContentRepository(trx); const resolvedId = (await resolveId(repo, collection, id)) ?? id; - return repo.unpublish(collection, resolvedId); + return repo.unpublish(collection, resolvedId, expectedRevision); }); const hasSeo = await collectionHasSeo(db, collection); @@ -1720,9 +1738,15 @@ export async function handleContentUnpublish( return { success: true, - data: { item }, + data: { item, _rev: encodeRev(item) }, }; } catch (error) { + if (error instanceof ContentMutationConflictError) { + return { + success: false, + error: { code: "CONFLICT", message: error.message }, + }; + } if (error instanceof EmDashValidationError) { return { success: false, @@ -1777,12 +1801,14 @@ export async function handleContentDiscardDraft( db: Kysely, collection: string, id: string, + options: { _rev?: string } = {}, ): Promise> { try { + const expectedRevision = decodeRevisionPrecondition(options._rev); const item = await withTransaction(db, async (trx) => { const repo = new ContentRepository(trx); const resolvedId = (await resolveId(repo, collection, id)) ?? id; - return repo.discardDraft(collection, resolvedId); + return repo.discardDraft(collection, resolvedId, expectedRevision); }); const hasSeo = await collectionHasSeo(db, collection); @@ -1790,9 +1816,15 @@ export async function handleContentDiscardDraft( return { success: true, - data: { item }, + data: { item, _rev: encodeRev(item) }, }; } catch (error) { + if (error instanceof ContentMutationConflictError) { + return { + success: false, + error: { code: "CONFLICT", message: error.message }, + }; + } if (error instanceof EmDashValidationError) { return { success: false, diff --git a/packages/core/src/api/schemas/content.ts b/packages/core/src/api/schemas/content.ts index f2142ad99f..796d07d7d8 100644 --- a/packages/core/src/api/schemas/content.ts +++ b/packages/core/src/api/schemas/content.ts @@ -219,8 +219,15 @@ export const contentScheduleBody = z }) .meta({ id: "ContentScheduleBody" }); -export const contentPublishBody = z - .object({ +export const contentRevisionConditionBody = z.object({ + _rev: z + .string() + .optional() + .meta({ description: "Opaque revision token for optimistic concurrency" }), +}); + +export const contentPublishBody = contentRevisionConditionBody + .extend({ // .optional() rather than .nullish(): publishing has no semantic // meaning for `null` (you can't "clear" a publish timestamp by // publishing). Tightening the schema here means callers either diff --git a/packages/core/src/astro/routes/api/content/[collection]/[id]/discard-draft.ts b/packages/core/src/astro/routes/api/content/[collection]/[id]/discard-draft.ts index 8433a60aec..70574e2d00 100644 --- a/packages/core/src/astro/routes/api/content/[collection]/[id]/discard-draft.ts +++ b/packages/core/src/astro/routes/api/content/[collection]/[id]/discard-draft.ts @@ -8,10 +8,12 @@ import type { APIRoute } from "astro"; import { requireOwnerPerm } from "#api/authorize.js"; import { apiError, mapErrorStatus, unwrapResult } from "#api/error.js"; +import { isParseError, parseOptionalBody } from "#api/parse.js"; +import { contentRevisionConditionBody } from "#api/schemas.js"; export const prerender = false; -export const POST: APIRoute = async ({ params, locals, url, cache }) => { +export const POST: APIRoute = async ({ params, request, locals, url, cache }) => { const { emdash, user } = locals; const collection = params.collection!; const id = params.id!; @@ -19,6 +21,8 @@ export const POST: APIRoute = async ({ params, locals, url, cache }) => { if (!emdash?.handleContentDiscardDraft || !emdash?.handleContentGet) { return apiError("NOT_CONFIGURED", "EmDash is not initialized", 500); } + const body = await parseOptionalBody(request, contentRevisionConditionBody, {}); + if (isParseError(body)) return body; const locale = url.searchParams.get("locale") || undefined; @@ -48,7 +52,9 @@ export const POST: APIRoute = async ({ params, locals, url, cache }) => { const resolvedId = typeof existingItem?.id === "string" ? existingItem.id : id; - const result = await emdash.handleContentDiscardDraft(collection, resolvedId); + const result = await emdash.handleContentDiscardDraft(collection, resolvedId, { + _rev: body?._rev, + }); if (!result.success) return unwrapResult(result); diff --git a/packages/core/src/astro/routes/api/content/[collection]/[id]/publish.ts b/packages/core/src/astro/routes/api/content/[collection]/[id]/publish.ts index 636e5f64a6..89ed747f40 100644 --- a/packages/core/src/astro/routes/api/content/[collection]/[id]/publish.ts +++ b/packages/core/src/astro/routes/api/content/[collection]/[id]/publish.ts @@ -3,7 +3,7 @@ * * POST /_emdash/api/content/{collection}/{id}/publish * - * Optional JSON body: { publishedAt?: string } + * Optional JSON body: { publishedAt?: string, _rev?: string } * publishedAt — ISO 8601 datetime to backdate the publish (e.g. when * migrating content). Writing publishedAt requires content:publish_any. * Without it, the existing published_at is preserved on re-publish and @@ -78,6 +78,7 @@ export const POST: APIRoute = async ({ params, request, locals, url, cache }) => const result = await emdash.handleContentPublish(collection, resolvedId, { publishedAt, + _rev: body?._rev, }); if (!result.success) return unwrapResult(result); diff --git a/packages/core/src/astro/routes/api/content/[collection]/[id]/unpublish.ts b/packages/core/src/astro/routes/api/content/[collection]/[id]/unpublish.ts index 5f62e06523..41ecc756c1 100644 --- a/packages/core/src/astro/routes/api/content/[collection]/[id]/unpublish.ts +++ b/packages/core/src/astro/routes/api/content/[collection]/[id]/unpublish.ts @@ -8,10 +8,12 @@ import type { APIRoute } from "astro"; import { requireOwnerPerm } from "#api/authorize.js"; import { apiError, mapErrorStatus, unwrapResult } from "#api/error.js"; +import { isParseError, parseOptionalBody } from "#api/parse.js"; +import { contentRevisionConditionBody } from "#api/schemas.js"; export const prerender = false; -export const POST: APIRoute = async ({ params, locals, url, cache }) => { +export const POST: APIRoute = async ({ params, request, locals, url, cache }) => { const { emdash, user } = locals; const collection = params.collection!; const id = params.id!; @@ -19,6 +21,8 @@ export const POST: APIRoute = async ({ params, locals, url, cache }) => { if (!emdash?.handleContentUnpublish || !emdash?.handleContentGet) { return apiError("NOT_CONFIGURED", "EmDash is not initialized", 500); } + const body = await parseOptionalBody(request, contentRevisionConditionBody, {}); + if (isParseError(body)) return body; const locale = url.searchParams.get("locale") || undefined; @@ -48,7 +52,7 @@ export const POST: APIRoute = async ({ params, locals, url, cache }) => { const resolvedId = typeof existingItem?.id === "string" ? existingItem.id : id; - const result = await emdash.handleContentUnpublish(collection, resolvedId); + const result = await emdash.handleContentUnpublish(collection, resolvedId, { _rev: body?._rev }); if (!result.success) return unwrapResult(result); diff --git a/packages/core/src/astro/types.ts b/packages/core/src/astro/types.ts index 869bbcff89..34ba4f47e9 100644 --- a/packages/core/src/astro/types.ts +++ b/packages/core/src/astro/types.ts @@ -350,10 +350,14 @@ export interface EmDashHandlers { handleContentPublish: ( collection: string, id: string, - options?: { publishedAt?: string; requireScheduledDue?: boolean }, + options?: { publishedAt?: string; requireScheduledDue?: boolean; _rev?: string }, ) => Promise; - handleContentUnpublish: (collection: string, id: string) => Promise; + handleContentUnpublish: ( + collection: string, + id: string, + options?: { _rev?: string }, + ) => Promise; handleContentSchedule: ( collection: string, @@ -365,7 +369,11 @@ export interface EmDashHandlers { handleContentCountScheduled: (collection: string) => Promise; - handleContentDiscardDraft: (collection: string, id: string) => Promise; + handleContentDiscardDraft: ( + collection: string, + id: string, + options?: { _rev?: string }, + ) => Promise; handleContentCompare: (collection: string, id: string) => Promise; diff --git a/packages/core/src/database/repositories/content.ts b/packages/core/src/database/repositories/content.ts index 9a15eb4c79..8d5bb279f8 100644 --- a/packages/core/src/database/repositories/content.ts +++ b/packages/core/src/database/repositories/content.ts @@ -42,6 +42,16 @@ const MAX_IN_FILTER_VALUES = SQL_BATCH_SIZE; const MAX_FILTER_STRING_LENGTH = 2048; type NormalizedFilterScalar = string | number; +export type ContentRevisionPrecondition = Pick; + +function assertRevisionPrecondition( + item: ContentItem, + expected: ContentRevisionPrecondition | undefined, +): void { + if (expected && (item.version !== expected.version || item.updatedAt !== expected.updatedAt)) { + throw new ContentMutationConflictError(); + } +} type ResolvedFieldFilter = | { column: string; kind: "null" } @@ -1777,6 +1787,7 @@ export class ContentRepository { expectedScheduledAt?: string, promoteRevision = true, requireSlug = true, + expectedRevision?: ContentRevisionPrecondition, ): Promise { const tableName = getTableName(type); const now = new Date().toISOString(); @@ -1785,6 +1796,7 @@ export class ContentRepository { if (!existing) { throw new EmDashValidationError("Content item not found"); } + assertRevisionPrecondition(existing, expectedRevision); if ( requireDue && expectedScheduledAt !== undefined && @@ -1829,6 +1841,7 @@ export class ContentRepository { WHERE id = ${id} AND deleted_at IS NULL AND version = ${existing.version} + AND updated_at = ${existing.updatedAt} AND status = ${existing.status} AND ${nullableColumnMatch("live_revision_id", existing.liveRevisionId)} AND ${nullableColumnMatch("draft_revision_id", existing.draftRevisionId)} @@ -1954,6 +1967,7 @@ export class ContentRepository { WHERE id = ${id} AND deleted_at IS NULL AND version = ${existing.version} + AND updated_at = ${existing.updatedAt} AND status = ${existing.status} AND ${nullableColumnMatch("live_revision_id", existing.liveRevisionId)} AND ${nullableColumnMatch("draft_revision_id", existing.draftRevisionId)} @@ -2031,7 +2045,11 @@ export class ContentRepository { * Removes live pointer but preserves draft. If no draft exists, * creates one from the live version so the content isn't lost. */ - async unpublish(type: string, id: string): Promise { + async unpublish( + type: string, + id: string, + expectedRevision?: ContentRevisionPrecondition, + ): Promise { const tableName = getTableName(type); const now = new Date().toISOString(); @@ -2039,44 +2057,62 @@ export class ContentRepository { if (!existing) { throw new EmDashValidationError("Content item not found"); } + assertRevisionPrecondition(existing, expectedRevision); + if (existing.status === "draft" && !existing.liveRevisionId) return existing; - // If no draft exists, create one from the live version - if (!existing.draftRevisionId && existing.liveRevisionId) { - const revisionRepo = new RevisionRepository(this.db); - const liveRevision = await revisionRepo.findById(existing.liveRevisionId); - if (liveRevision) { - const draft = await revisionRepo.create({ - collection: type, - entryId: id, - data: liveRevision.data, - }); - - await sql` - UPDATE ${sql.ref(tableName)} - SET draft_revision_id = ${draft.id} - WHERE id = ${id} - `.execute(this.db); + const revisionRepo = new RevisionRepository(this.db); + let provisionalRevisionId: string | null = null; + try { + let draftRevisionId = existing.draftRevisionId; + if (!draftRevisionId && existing.liveRevisionId) { + const liveRevision = await revisionRepo.findById(existing.liveRevisionId); + if (liveRevision) { + const draft = await revisionRepo.create({ + collection: type, + entryId: id, + data: liveRevision.data, + }); + draftRevisionId = draft.id; + provisionalRevisionId = draft.id; + } } - } - await sql` - UPDATE ${sql.ref(tableName)} - SET live_revision_id = NULL, - status = 'draft', - published_at = NULL, - updated_at = ${now} - WHERE id = ${id} - AND deleted_at IS NULL - `.execute(this.db); + const result = await sql` + UPDATE ${sql.ref(tableName)} + SET live_revision_id = NULL, + draft_revision_id = ${draftRevisionId}, + status = 'draft', + published_at = NULL, + updated_at = ${now}, + version = version + 1 + WHERE id = ${id} + AND deleted_at IS NULL + AND version = ${existing.version} + AND updated_at = ${existing.updatedAt} + AND status = ${existing.status} + AND ${nullableColumnMatch("live_revision_id", existing.liveRevisionId)} + AND ${nullableColumnMatch("draft_revision_id", existing.draftRevisionId)} + AND ${nullableColumnMatch("scheduled_at", existing.scheduledAt)} + `.execute(this.db); + if ((result.numAffectedRows ?? 0n) === 0n) throw new ContentMutationConflictError(); - invalidateCollectionCache(type); - - const updated = await this.findById(type, id); - if (!updated) { - throw new Error("Content not found"); + invalidateCollectionCache(type); + const updated = await this.findById(type, id); + if (!updated) throw new Error("Content not found"); + return updated; + } catch (error) { + if (provisionalRevisionId) { + try { + await revisionRepo.deleteIfUnreferenced(type, id, provisionalRevisionId); + } catch (cleanupError) { + console.error( + `[content] Failed to clean up provisional revision ${provisionalRevisionId}:`, + cleanupError, + ); + } + } + throw error; } - - return updated; } /** @@ -2143,13 +2179,18 @@ export class ContentRepository { * Clears draft_revision_id. The content table columns already hold the * published version, so no data sync is needed. */ - async discardDraft(type: string, id: string): Promise { + async discardDraft( + type: string, + id: string, + expectedRevision?: ContentRevisionPrecondition, + ): Promise { const tableName = getTableName(type); const existing = await this.findById(type, id); if (!existing) { throw new EmDashValidationError("Content item not found"); } + assertRevisionPrecondition(existing, expectedRevision); if (!existing.draftRevisionId) { // No draft to discard @@ -2159,12 +2200,20 @@ export class ContentRepository { // Discarding a draft restores the state from before the draft was // staged — nothing about the live entry changed in between, so // updated_at stays at its pre-draft value (#2143). - await sql` + const result = await sql` UPDATE ${sql.ref(tableName)} - SET draft_revision_id = NULL + SET draft_revision_id = NULL, + version = version + 1 WHERE id = ${id} AND deleted_at IS NULL + AND version = ${existing.version} + AND updated_at = ${existing.updatedAt} + AND status = ${existing.status} + AND ${nullableColumnMatch("live_revision_id", existing.liveRevisionId)} + AND ${nullableColumnMatch("draft_revision_id", existing.draftRevisionId)} + AND ${nullableColumnMatch("scheduled_at", existing.scheduledAt)} `.execute(this.db); + if ((result.numAffectedRows ?? 0n) === 0n) throw new ContentMutationConflictError(); const updated = await this.findById(type, id); if (!updated) { diff --git a/packages/core/src/emdash-runtime.ts b/packages/core/src/emdash-runtime.ts index 9097d6847e..2e0a83707e 100644 --- a/packages/core/src/emdash-runtime.ts +++ b/packages/core/src/emdash-runtime.ts @@ -3327,6 +3327,7 @@ export class EmDashRuntime { publishedAt?: string; requireScheduledDue?: boolean; expectedScheduledAt?: string; + _rev?: string; } = {}, ) { const result = await handleContentPublish(this.db, collection, id, options); @@ -3342,8 +3343,8 @@ export class EmDashRuntime { return result; } - async handleContentUnpublish(collection: string, id: string) { - const result = await handleContentUnpublish(this.db, collection, id); + async handleContentUnpublish(collection: string, id: string, options: { _rev?: string } = {}) { + const result = await handleContentUnpublish(this.db, collection, id, options); if (result.success && result.data) { await this.refreshContentUsageAfterSuccessfulWrite(collection, [result.data.item.id]); } @@ -3388,8 +3389,8 @@ export class EmDashRuntime { return handleContentCountScheduled(this.db, collection); } - async handleContentDiscardDraft(collection: string, id: string) { - const result = await handleContentDiscardDraft(this.db, collection, id); + async handleContentDiscardDraft(collection: string, id: string, options: { _rev?: string } = {}) { + const result = await handleContentDiscardDraft(this.db, collection, id, options); if (result.success && result.data) { await this.refreshContentUsageAfterSuccessfulWrite(collection, [result.data.item.id]); } diff --git a/packages/core/src/mcp/server.ts b/packages/core/src/mcp/server.ts index b69ec92f26..3140630238 100644 --- a/packages/core/src/mcp/server.ts +++ b/packages/core/src/mcp/server.ts @@ -1260,6 +1260,10 @@ export function createMcpServer( inputSchema: z.object({ collection: z.string().describe("Collection slug"), id: z.string().describe("Content item ID or slug"), + _rev: z + .string() + .optional() + .describe("Revision token from content_get for conflict detection"), publishedAt: z.iso .datetime({ offset: true, message: "must be an ISO 8601 datetime" }) .optional() @@ -1297,6 +1301,7 @@ export function createMcpServer( return unwrap( await emdash.handleContentPublish(args.collection, resolvedId, { publishedAt: args.publishedAt, + _rev: args._rev, }), ); }, @@ -1312,6 +1317,10 @@ export function createMcpServer( inputSchema: z.object({ collection: z.string().describe("Collection slug"), id: z.string().describe("Content item ID or slug"), + _rev: z + .string() + .optional() + .describe("Revision token from content_get for conflict detection"), }), }, async (args, extra) => { @@ -1332,7 +1341,9 @@ export function createMcpServer( ); const resolvedId = extractContentId(existing.data) ?? args.id; - return unwrap(await ec.handleContentUnpublish(args.collection, resolvedId)); + return unwrap( + await ec.handleContentUnpublish(args.collection, resolvedId, { _rev: args._rev }), + ); }, ); @@ -1440,6 +1451,10 @@ export function createMcpServer( inputSchema: z.object({ collection: z.string().describe("Collection slug"), id: z.string().describe("Content item ID or slug"), + _rev: z + .string() + .optional() + .describe("Revision token from content_get for conflict detection"), }), annotations: { destructiveHint: true }, }, @@ -1461,7 +1476,9 @@ export function createMcpServer( ); const resolvedId = extractContentId(existing.data) ?? args.id; - return unwrap(await ec.handleContentDiscardDraft(args.collection, resolvedId)); + return unwrap( + await ec.handleContentDiscardDraft(args.collection, resolvedId, { _rev: args._rev }), + ); }, ); diff --git a/packages/core/tests/integration/mcp/publish-revision-cas.test.ts b/packages/core/tests/integration/mcp/publish-revision-cas.test.ts new file mode 100644 index 0000000000..79015a607f --- /dev/null +++ b/packages/core/tests/integration/mcp/publish-revision-cas.test.ts @@ -0,0 +1,191 @@ +import { Role } from "@emdash-cms/auth"; +import type { Kysely } from "kysely"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import type { Database } from "../../../src/database/types.js"; +import { + connectMcpHarness, + extractJson, + extractText, + type McpHarness, +} from "../../utils/mcp-runtime.js"; +import { setupTestDatabaseWithCollections, teardownTestDatabase } from "../../utils/test-db.js"; + +const ADMIN_ID = "user_admin"; + +interface ContentResult { + item: { id: string; status: string; data: { title?: string } }; + _rev: string; +} + +describe("MCP conditional content publication", () => { + let db: Kysely; + let harness: McpHarness; + + beforeEach(async () => { + db = await setupTestDatabaseWithCollections(); + harness = await connectMcpHarness({ db, userId: ADMIN_ID, userRole: Role.ADMIN }); + }); + + afterEach(async () => { + if (harness) await harness.cleanup(); + await teardownTestDatabase(db); + }); + + async function createDraft(title = "Draft"): Promise { + const result = await harness.client.callTool({ + name: "content_create", + arguments: { collection: "post", data: { title }, slug: title.toLowerCase() }, + }); + expect(result.isError, extractText(result)).toBeFalsy(); + return extractJson(result); + } + + async function createPublished(): Promise { + const created = await createDraft(); + const result = await harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id }, + }); + expect(result.isError, extractText(result)).toBeFalsy(); + return extractJson(result); + } + + async function update(id: string, rev: string, title: string): Promise { + const result = await harness.client.callTool({ + name: "content_update", + arguments: { collection: "post", id, data: { title }, _rev: rev }, + }); + expect(result.isError, extractText(result)).toBeFalsy(); + return extractJson(result); + } + + it("M01 publishes with the current _rev", async () => { + const created = await createDraft(); + const result = await harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id, _rev: created._rev }, + }); + + expect(result.isError, extractText(result)).toBeFalsy(); + expect(extractJson(result).item.status).toBe("published"); + }); + + it("M02 rejects a stale _rev after a newer draft save", async () => { + const published = await createPublished(); + const approved = await update(published.item.id, published._rev, "Approved"); + await update(published.item.id, approved._rev, "Writer change"); + + const result = await harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: published.item.id, _rev: approved._rev }, + }); + + expect(result.isError).toBe(true); + expect(extractText(result)).toMatch(/CONFLICT/); + }); + + it("M03 rejects content_unpublish with a stale _rev", async () => { + const published = await createPublished(); + await update(published.item.id, published._rev, "Pending"); + + const result = await harness.client.callTool({ + name: "content_unpublish", + arguments: { collection: "post", id: published.item.id, _rev: published._rev }, + }); + + expect(result.isError).toBe(true); + expect(extractText(result)).toMatch(/CONFLICT/); + }); + + it("M04 rejects content_discard_draft with a stale _rev", async () => { + const published = await createPublished(); + await update(published.item.id, published._rev, "Pending"); + + const result = await harness.client.callTool({ + name: "content_discard_draft", + arguments: { collection: "post", id: published.item.id, _rev: published._rev }, + }); + + expect(result.isError).toBe(true); + expect(extractText(result)).toMatch(/CONFLICT/); + }); + + it("M05 keeps revisionless publish backward compatible", async () => { + const created = await createDraft(); + const result = await harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id }, + }); + + expect(result.isError, extractText(result)).toBeFalsy(); + }); + + it("M06 rejects a caller without content:write even with a valid _rev", async () => { + const created = await createDraft(); + const restricted = await connectMcpHarness({ + db, + userId: ADMIN_ID, + userRole: Role.ADMIN, + tokenScopes: ["content:read"], + }); + + try { + const result = await restricted.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id, _rev: created._rev }, + }); + expect(result.isError).toBe(true); + expect(extractText(result)).toMatch(/scope|permission/i); + } finally { + await restricted.cleanup(); + } + }); + + it("M07 rejects a malformed _rev without publishing", async () => { + const created = await createDraft(); + const result = await harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id, _rev: "not-a-revision" }, + }); + + expect(result.isError).toBe(true); + expect(extractText(result)).toMatch(/CONFLICT/); + const current = await harness.client.callTool({ + name: "content_get", + arguments: { collection: "post", id: created.item.id }, + }); + expect(extractJson(current).item.status).toBe("draft"); + }); + + it("M08 returns a new _rev after publish", async () => { + const created = await createDraft(); + const result = await harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id, _rev: created._rev }, + }); + const published = extractJson(result); + + expect(published._rev).toBeTruthy(); + expect(published._rev).not.toBe(created._rev); + }); + + it("M09 allows only one concurrent publish for the same _rev", async () => { + const created = await createDraft(); + const results = await Promise.all([ + harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id, _rev: created._rev }, + }), + harness.client.callTool({ + name: "content_publish", + arguments: { collection: "post", id: created.item.id, _rev: created._rev }, + }), + ]); + + expect(results.filter((result) => !result.isError)).toHaveLength(1); + expect( + results.filter((result) => result.isError && /CONFLICT/.test(extractText(result))), + ).toHaveLength(1); + }); +}); diff --git a/packages/core/tests/unit/api/publish-revision-cas.test.ts b/packages/core/tests/unit/api/publish-revision-cas.test.ts new file mode 100644 index 0000000000..5601ba4254 --- /dev/null +++ b/packages/core/tests/unit/api/publish-revision-cas.test.ts @@ -0,0 +1,231 @@ +import { Role } from "@emdash-cms/auth"; +import type { APIContext } from "astro"; +import type { Kysely } from "kysely"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { POST as postPublish } from "../../../src/astro/routes/api/content/[collection]/[id]/publish.js"; +import type { Database } from "../../../src/database/types.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, + setupForDialectWithCollections, + teardownForDialect, + type DialectTestContext, +} from "../../utils/test-db.js"; + +describeEachDialect("conditional content publication", (dialect) => { + let ctx: DialectTestContext; + let db: Kysely; + let runtime: EmDashRuntime; + + beforeEach(async () => { + ctx = await setupForDialectWithCollections(dialect); + db = ctx.db; + runtime = createTestRuntime(db); + }); + + afterEach(async () => { + await teardownForDialect(ctx); + }); + + async function createPublished(title = "Live") { + const created = await runtime.handleContentCreate("post", { + data: { title }, + slug: title.toLowerCase(), + }); + expect(created.success).toBe(true); + const published = await runtime.handleContentPublish("post", created.data!.item.id); + expect(published.success).toBe(true); + return published.data!; + } + + it("publishes only the expected revision and returns the next _rev", async () => { + const created = await createPublished(); + const saved = await runtime.handleContentUpdate("post", created.item.id, { + data: { title: "Approved" }, + _rev: created._rev, + }); + expect(saved.success).toBe(true); + + const published = await runtime.handleContentPublish("post", created.item.id, { + _rev: saved.data!._rev, + }); + + expect(published.success).toBe(true); + expect(published.data!.item.data.title).toBe("Approved"); + expect(published.data!._rev).toBeTruthy(); + expect(published.data!._rev).not.toBe(saved.data!._rev); + + const nextSave = await runtime.handleContentUpdate("post", created.item.id, { + data: { title: "Next" }, + _rev: published.data!._rev, + }); + expect(nextSave.success).toBe(true); + }); + + it("rejects an approved revision after a newer draft save without mutating live or draft", async () => { + const created = await createPublished(); + const approved = await runtime.handleContentUpdate("post", created.item.id, { + data: { title: "Approved A" }, + _rev: created._rev, + }); + const newer = await runtime.handleContentUpdate("post", created.item.id, { + data: { title: "Writer B" }, + _rev: approved.data!._rev, + skipRevision: true, + }); + expect(newer.data!.item.draftRevisionId).not.toBe(approved.data!.item.draftRevisionId); + const before = await runtime.handleContentGet("post", created.item.id); + + const stale = await runtime.handleContentPublish("post", created.item.id, { + _rev: approved.data!._rev, + }); + const after = await runtime.handleContentGet("post", created.item.id); + + expect(stale).toMatchObject({ success: false, error: { code: "CONFLICT" } }); + expect(after.data!.item.liveRevisionId).toBe(before.data!.item.liveRevisionId); + expect(after.data!.item.draftRevisionId).toBe(before.data!.item.draftRevisionId); + expect(after.data!._rev).toBe(newer.data!._rev); + }); + + it("allows only one of two concurrent publishes for the same _rev", async () => { + const created = await createPublished(); + const saved = await runtime.handleContentUpdate("post", created.item.id, { + data: { title: "Approved" }, + _rev: created._rev, + }); + + const results = await Promise.all([ + runtime.handleContentPublish("post", created.item.id, { _rev: saved.data!._rev }), + runtime.handleContentPublish("post", created.item.id, { _rev: saved.data!._rev }), + ]); + + expect(results.filter((result) => result.success)).toHaveLength(1); + expect( + results.filter((result) => !result.success && result.error.code === "CONFLICT"), + ).toHaveLength(1); + const published = await runtime.handleContentGet("post", created.item.id); + expect(published.data!.item.data.title).toBe("Approved"); + expect(published.data!.item.draftRevisionId).toBeNull(); + }); + + it("supports conditional publish for a collection without revisions", async () => { + const registry = new SchemaRegistry(db); + await registry.updateCollection("post", { supports: [] }); + const created = await createPublished(); + + const published = await runtime.handleContentPublish("post", created.item.id, { + _rev: created._rev, + }); + + expect(published.success).toBe(true); + expect(published.data!._rev).not.toBe(created._rev); + }); + + it("applies the same revision condition to unpublish and discard-draft", async () => { + const created = await createPublished(); + const saved = await runtime.handleContentUpdate("post", created.item.id, { + data: { title: "Pending" }, + _rev: created._rev, + }); + + const staleUnpublish = await runtime.handleContentUnpublish("post", created.item.id, { + _rev: created._rev, + }); + const staleDiscard = await runtime.handleContentDiscardDraft("post", created.item.id, { + _rev: created._rev, + }); + expect(staleUnpublish).toMatchObject({ success: false, error: { code: "CONFLICT" } }); + expect(staleDiscard).toMatchObject({ success: false, error: { code: "CONFLICT" } }); + + const discarded = await runtime.handleContentDiscardDraft("post", created.item.id, { + _rev: saved.data!._rev, + }); + expect(discarded.success).toBe(true); + expect(discarded.data!._rev).not.toBe(saved.data!._rev); + }); + + it("keeps revisionless publish calls backward compatible", async () => { + const created = await createPublished(); + const published = await runtime.handleContentPublish("post", created.item.id); + + expect(published.success).toBe(true); + expect(published.data!._rev).toBeTruthy(); + }); + + it("rejects malformed revision conditions as conflicts", async () => { + const created = await createPublished(); + const published = await runtime.handleContentPublish("post", created.item.id, { + _rev: "not-a-revision", + }); + + expect(published).toMatchObject({ success: false, error: { code: "CONFLICT" } }); + }); +}); + +describe("conditional publish route authorization", () => { + it("passes the revision condition through and returns the next revision", async () => { + const handleContentPublish = vi.fn().mockResolvedValue({ + success: true, + data: { item: { id: "entry", authorId: "owner" }, _rev: "next-rev" }, + }); + const request = new Request("http://localhost/_emdash/api/content/post/entry/publish", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ _rev: "approved-rev" }), + }); + const response = await postPublish({ + params: { collection: "post", id: "entry" }, + request, + url: new URL(request.url), + locals: { + user: { id: "owner", role: Role.EDITOR }, + emdash: { + handleContentGet: vi.fn().mockResolvedValue({ + success: true, + data: { item: { id: "entry", authorId: "owner" }, _rev: "approved-rev" }, + }), + handleContentPublish, + }, + }, + cache: { enabled: false, invalidate: vi.fn() }, + } as unknown as APIContext); + + expect(response.status).toBe(200); + expect(handleContentPublish).toHaveBeenCalledWith("post", "entry", { + publishedAt: undefined, + _rev: "approved-rev", + }); + expect(await response.json()).toMatchObject({ data: { _rev: "next-rev" } }); + }); + + it("does not let an unprivileged caller publish even with the current _rev", async () => { + const handleContentPublish = vi.fn(); + const request = new Request("http://localhost/_emdash/api/content/post/entry/publish", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ _rev: "current-rev" }), + }); + const response = await postPublish({ + params: { collection: "post", id: "entry" }, + request, + url: new URL(request.url), + locals: { + user: { id: "subscriber", role: Role.SUBSCRIBER }, + emdash: { + handleContentGet: vi.fn().mockResolvedValue({ + success: true, + data: { item: { id: "entry", authorId: "subscriber" }, _rev: "current-rev" }, + }), + handleContentPublish, + }, + }, + cache: { enabled: false, invalidate: vi.fn() }, + } as unknown as APIContext); + + expect(response.status).toBe(403); + expect(handleContentPublish).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/tests/unit/mcp/authorization.test.ts b/packages/core/tests/unit/mcp/authorization.test.ts index a23c637068..492492ff8b 100644 --- a/packages/core/tests/unit/mcp/authorization.test.ts +++ b/packages/core/tests/unit/mcp/authorization.test.ts @@ -966,7 +966,9 @@ describe("MCP Authorization", () => { }); expect(result.isError).toBeFalsy(); - expect(handlers.handleContentDiscardDraft).toHaveBeenCalledWith("post", CONTENT_ID); + expect(handlers.handleContentDiscardDraft).toHaveBeenCalledWith("post", CONTENT_ID, { + _rev: undefined, + }); }); it("content_update passes resolvedId (not slug) to handler", async () => {