From 3cc8ffe8c59069da538da31bb2b62a84f2285f0f Mon Sep 17 00:00:00 2001 From: Alice Alexandra Moore <86723305+3mdistal@users.noreply.github.com> Date: Wed, 9 Sep 2026 12:47:54 -0400 Subject: [PATCH 1/3] fix: reject trashed Content writes and recover restored editors --- .changeset/quiet-documents-stay-quiet.md | 7 + .../core/src/collab/client.registry.spec.tsx | 125 +++++++++ packages/core/src/collab/client.ts | 77 ++++-- packages/core/src/collab/index.ts | 6 + packages/core/src/collab/lifecycle.spec.ts | 170 ++++++++++++ packages/core/src/collab/lifecycle.ts | 95 +++++++ packages/core/src/collab/storage.ts | 59 +++-- packages/core/src/collab/ydoc-manager.spec.ts | 85 ++++++ packages/core/src/collab/ydoc-manager.ts | 30 +-- .../core/src/server/collab-plugin.spec.ts | 120 +++++++++ packages/core/src/server/collab-plugin.ts | 246 ++++++++++-------- .../actions/_database-block-actions.ts | 33 +-- .../_document-edit-mutation.db.test.ts | 44 +++- .../actions/_document-edit-mutation.ts | 12 +- .../content/actions/_document-lifecycle.ts | 42 +++ .../actions/_document-mutation-access.ts | 103 ++++++++ templates/content/actions/add-comment.test.ts | 3 + templates/content/actions/add-comment.ts | 4 + .../content/actions/blocks-seeding.db.test.ts | 194 ++++++++++++++ .../actions/comment-submission.db.test.ts | 68 +++++ templates/content/actions/delete-comment.ts | 22 +- .../actions/document-history.db.test.ts | 66 +++++ templates/content/actions/edit-document.ts | 8 + templates/content/actions/get-document.ts | 5 +- .../actions/restore-document-version.ts | 74 +++--- .../content/actions/set-document-property.ts | 29 +++ .../content/actions/update-comment.test.ts | 3 + templates/content/actions/update-comment.ts | 27 +- .../actions/update-document.db.test.ts | 114 ++++++++ templates/content/actions/update-document.ts | 73 +++++- .../editor/DocumentEditor.layout.test.ts | 71 ++++- .../app/components/editor/DocumentEditor.tsx | 68 ++++- .../app/components/editor/DocumentToolbar.tsx | 9 +- .../editor/PageDraftRecovery.test.tsx | 62 ++++- .../components/editor/PageDraftRecovery.tsx | 62 ++++- .../editor/database/DatabaseView.tsx | 10 +- .../app/hooks/content-action-refresh.ts | 47 +++- .../content/app/hooks/use-db-sync.spec.ts | 60 +++++ .../content/app/hooks/use-documents.test.ts | 85 ++++++ templates/content/app/hooks/use-documents.ts | 114 +++++--- templates/content/app/i18n-data.ts | 10 + templates/content/app/i18n/zh-TW.ts | 1 + .../app/lib/document-fetch-state.test.ts | 68 +++++ .../content/app/lib/document-fetch-state.ts | 29 +++ templates/content/server/plugins/collab.ts | 1 + 45 files changed, 2316 insertions(+), 325 deletions(-) create mode 100644 .changeset/quiet-documents-stay-quiet.md create mode 100644 packages/core/src/collab/lifecycle.spec.ts create mode 100644 packages/core/src/collab/lifecycle.ts create mode 100644 templates/content/actions/_document-lifecycle.ts create mode 100644 templates/content/actions/_document-mutation-access.ts create mode 100644 templates/content/app/lib/document-fetch-state.test.ts create mode 100644 templates/content/app/lib/document-fetch-state.ts diff --git a/.changeset/quiet-documents-stay-quiet.md b/.changeset/quiet-documents-stay-quiet.md new file mode 100644 index 00000000000..a1b369021d7 --- /dev/null +++ b/.changeset/quiet-documents-stay-quiet.md @@ -0,0 +1,7 @@ +--- +"@agent-native/core": patch +--- + +Allow collaboration persistence to enforce the source document's live lifecycle within the same transaction, and discard cached mutations when a write fails. + +Quarantine client connections after terminal document lifecycle rejections so retries and remounts fetch authoritative state without replaying rejected edits. diff --git a/packages/core/src/collab/client.registry.spec.tsx b/packages/core/src/collab/client.registry.spec.tsx index b73a2c2d5c1..4e36a095231 100644 --- a/packages/core/src/collab/client.registry.spec.tsx +++ b/packages/core/src/collab/client.registry.spec.tsx @@ -586,4 +586,129 @@ describe("useCollaborativeDoc connection registry", () => { ).length; expect(awarenessPostsAfterDispose).toBe(awarenessPostsBeforeDispose); }); + it.each(["DOCUMENT_TRASHED", "DOCUMENT_NOT_FOUND"] as const)( + "quarantines %s and retries with authoritative state instead of rejected updates", + async (errorCode) => { + const { mock: fallback, stateFetches } = makeFetchMock(); + let rejectUpdate!: (response: Response) => void; + let updatePosts = 0; + vi.stubGlobal( + "fetch", + vi.fn((input: RequestInfo | URL) => { + if (String(input).endsWith("/update")) { + updatePosts++; + return new Promise((resolve) => { + rejectUpdate = resolve; + }); + } + return fallback(input); + }), + ); + let result: UseCollaborativeDocResult | undefined; + const root = mount( + (result = r)} />, + ); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + const dirtyDoc = result!.ydoc!; + act(() => dirtyDoc.getText("content").insert(0, "interrupted ")); + await act(async () => { + await vi.advanceTimersByTimeAsync(100); + }); + act(() => dirtyDoc.getText("content").insert(0, "queued ")); + await act(async () => { + await vi.advanceTimersByTimeAsync(100); + }); + expect(updatePosts).toBe(1); + await act(async () => { + rejectUpdate( + new Response(JSON.stringify({ data: { errorCode } }), { + status: errorCode === "DOCUMENT_TRASHED" ? 409 : 404, + }), + ); + await vi.advanceTimersByTimeAsync(0); + }); + expect(result!.initialization).toEqual({ + status: "error", + category: "forbidden-or-not-found", + errorCode, + }); + expect(result!.ydoc).toBeNull(); + expect(dirtyDoc.isDestroyed).toBe(false); + expect(dirtyDoc.getText("content").toString()).toBe( + "queued interrupted seed", + ); + expect(_collabDocRegistrySizeForTests()).toBe(0); + act(() => dirtyDoc.getText("content").insert(0, "retained ")); + act(() => window.dispatchEvent(new Event("pagehide"))); + await act(async () => { + await vi.advanceTimersByTimeAsync(100); + }); + expect(updatePosts).toBe(1); + if (errorCode === "DOCUMENT_TRASHED") { + act(() => result!.retry()); + } else { + act(() => root.unmount()); + roots = roots.filter((candidate) => candidate !== root); + mount( (result = r)} />); + } + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result!.ydoc).not.toBe(dirtyDoc); + expect(result!.ydoc!.getText("content").toString()).toBe("seed"); + expect(dirtyDoc.isDestroyed).toBe(true); + expect(stateFetches).toHaveLength(2); + expect(updatePosts).toBe(1); + }, + ); + it("evicts a lingered dirty connection when rejection arrives after unmount", async () => { + const { mock: fallback } = makeFetchMock(); + let rejectUpdate!: (response: Response) => void; + let updatePosts = 0; + vi.stubGlobal( + "fetch", + vi.fn((input: RequestInfo | URL) => { + if (String(input).endsWith("/update")) { + updatePosts++; + return new Promise((resolve) => { + rejectUpdate = resolve; + }); + } + return fallback(input); + }), + ); + let result: UseCollaborativeDocResult | undefined; + const root = mount( + (result = r)} />, + ); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + const dirtyDoc = result!.ydoc!; + act(() => dirtyDoc.getText("content").insert(0, "draft ")); + await act(async () => { + await vi.advanceTimersByTimeAsync(100); + }); + act(() => root.unmount()); + roots = roots.filter((candidate) => candidate !== root); + await act(async () => { + rejectUpdate( + new Response(JSON.stringify({ errorCode: "DOCUMENT_TRASHED" }), { + status: 409, + }), + ); + await vi.advanceTimersByTimeAsync(0); + }); + expect(_collabDocRegistrySizeForTests()).toBe(0); + expect(dirtyDoc.isDestroyed).toBe(true); + mount( (result = r)} />); + await act(async () => { + await vi.advanceTimersByTimeAsync(0); + }); + expect(result!.ydoc).not.toBe(dirtyDoc); + expect(result!.ydoc!.getText("content").toString()).toBe("seed"); + expect(updatePosts).toBe(1); + }); }); diff --git a/packages/core/src/collab/client.ts b/packages/core/src/collab/client.ts index cab1864f6e5..ef7954a1167 100644 --- a/packages/core/src/collab/client.ts +++ b/packages/core/src/collab/client.ts @@ -81,7 +81,11 @@ export type CollabInitializationErrorCategory = export type CollabInitializationState = | { status: "loading" } | { status: "ready" } - | { status: "error"; category: CollabInitializationErrorCategory }; + | { + status: "error"; + category: CollabInitializationErrorCategory; + errorCode?: "DOCUMENT_TRASHED" | "DOCUMENT_NOT_FOUND"; + }; export interface UseCollaborativeDocResult { /** The Yjs document instance. Stable per docId — never changes identity. */ @@ -345,6 +349,7 @@ class CollabDocConnection { /** Immutable snapshot of the shared reactive state (replaced on change). */ snapshot: CollabDocSnapshot; disposed = false; + quarantined = false; private subscribers = new Map(); private disposeTimer: ReturnType | null = null; @@ -355,6 +360,7 @@ class CollabDocConnection { // Local-update batching (debounced + coalesced with Y.mergeUpdates). private pendingUpdates: Uint8Array[] = []; + private updateInFlight = false; private flushTimer: ReturnType | null = null; private updateHandlerAttached = false; @@ -447,7 +453,8 @@ class CollabDocConnection { // live consumer while empty. Do not let a queued presence push escape // after the last subscriber has gone away. cancelAwarenessPush(this.baseUrl, this.docId, this.ydoc.clientID); - this.scheduleDispose(); + if (this.quarantined) this.dispose(); + else this.scheduleDispose(); } else { this.resubscribeCollabEventsIfPauseChanged(); this.reschedulePoll(); @@ -553,7 +560,7 @@ class CollabDocConnection { * multiple subscribers for the same user don't re-emit awareness updates. */ setUser(user: CollabUser): void { - if (this.disposed) return; + if (this.disposed || this.quarantined) return; const prev = this.lastSetUser; if ( prev && @@ -672,7 +679,7 @@ class CollabDocConnection { // even when the doc is missing (matches the previous per-hook behavior). this.unsubscribeAwarenessEvents = subscribeSyncEvents({ onEvents: (events) => { - if (this.disposed) return; + if (this.disposed || this.quarantined) return; for (const data of events) this.applyAwarenessEvent(data); }, }); @@ -681,7 +688,7 @@ class CollabDocConnection { private fetchInitialState(): void { fetch(`${this.baseUrl}/${this.docId}/state`).then( async (res) => { - if (this.disposed) return; + if (this.disposed || this.quarantined) return; if (res.status === 404 || res.status === 403) { this.markInitializationFailed("forbidden-or-not-found"); return; @@ -693,7 +700,7 @@ class CollabDocConnection { const data = (await res.json().catch(() => null)) as { state?: string; } | null; - if (this.disposed) return; + if (this.disposed || this.quarantined) return; if (typeof data?.state !== "string" || data.state.length === 0) { this.markInitializationFailed("invalid-payload"); return; @@ -725,7 +732,7 @@ class CollabDocConnection { this.startTransport(); }, () => { - if (this.disposed) return; + if (this.disposed || this.quarantined) return; this.markInitializationFailed("network"); }, ); @@ -738,6 +745,7 @@ class CollabDocConnection { */ private markInitializationFailed( category: CollabInitializationErrorCategory, + errorCode?: "DOCUMENT_TRASHED" | "DOCUMENT_NOT_FOUND", ): void { this.docMissing = true; this.pendingUpdates = []; @@ -752,12 +760,16 @@ class CollabDocConnection { this.setSnapshot({ isLoading: false, isSynced: false, - initialization: { status: "error", category }, + initialization: { + status: "error", + category, + ...(errorCode ? { errorCode } : {}), + }, }); } private retryInitialization(): void { - if (this.disposed) return; + if (this.disposed || this.quarantined) return; this.detachUpdateHandler(); this.stopSync(); this.unsubscribeAwarenessEvents?.(); @@ -780,7 +792,7 @@ class CollabDocConnection { // ------------------------------------------------------------------------- private handleDocUpdate = (update: Uint8Array, origin: unknown): void => { - if (origin === "remote") return; + if (origin === "remote" || this.quarantined || this.disposed) return; this.pendingUpdates.push(update); if (this.flushTimer) clearTimeout(this.flushTimer); this.flushTimer = setTimeout( @@ -816,11 +828,17 @@ class CollabDocConnection { clearTimeout(this.flushTimer); this.flushTimer = null; } - if (this.pendingUpdates.length === 0) return; + if ( + this.quarantined || + this.updateInFlight || + this.pendingUpdates.length === 0 + ) + return; const toSend = this.pendingUpdates; this.pendingUpdates = []; const merged = toSend.length === 1 ? toSend[0] : Y.mergeUpdates(toSend); + this.updateInFlight = true; fetch(`${this.baseUrl}/${this.docId}/update`, { method: "POST", headers: { "Content-Type": "application/json" }, @@ -829,7 +847,36 @@ class CollabDocConnection { requestSource: this.requestSource, }), ...(keepalive ? { keepalive: true } : {}), - }).catch(() => {}); + }) + .then(async (res) => { + if (res.ok || (res.status !== 409 && res.status !== 404)) return; + const payload = await res.json(); + const code = + payload?.errorCode ?? payload?.data?.errorCode ?? payload?.code; + if (code === "DOCUMENT_TRASHED" || code === "DOCUMENT_NOT_FOUND") { + this.quarantine(code); + } + }) + .catch(() => { + // Network failures remain recoverable through the application's save path. + }) + .finally(() => { + this.updateInFlight = false; + if (!this.quarantined) this.flushPendingUpdates(this.disposed); + }); + } + + private quarantine( + errorCode: "DOCUMENT_TRASHED" | "DOCUMENT_NOT_FOUND", + ): void { + this.quarantined = true; + if (collabConnectionRegistry.get(this.registryKey) === this) { + collabConnectionRegistry.delete(this.registryKey); + } + cancelAwarenessPush(this.baseUrl, this.docId, this.ydoc.clientID); + this.markInitializationFailed("forbidden-or-not-found", errorCode); + // Current consumers retain their rejected document for draft recovery. + if (this.subscribers.size === 0) this.dispose(); } // ------------------------------------------------------------------------- @@ -989,7 +1036,7 @@ class CollabDocConnection { const stateData = (await stateRes.json().catch(() => null)) as { state?: string; } | null; - if (this.disposed) return; + if (this.disposed || this.quarantined) return; if (stateData?.state) { const binary = base64ToUint8Array(stateData.state); if (binary.length > 2) { @@ -1096,7 +1143,7 @@ class CollabDocConnection { // Invalid state — skip } } - if (this.disposed) return; + if (this.disposed || this.quarantined) return; const changes = reconcileRemoteAwarenessStates( this.awareness.getStates() as Map, this.ydoc.clientID, @@ -1355,7 +1402,7 @@ export function useCollaborativeDoc( initialization: snapshot.initialization, retry: conn ? () => { - if (conn.disposed) { + if (conn.disposed || conn.quarantined) { setGeneration((current) => current + 1); return; } diff --git a/packages/core/src/collab/index.ts b/packages/core/src/collab/index.ts index 5b60874d3aa..31b904f468f 100644 --- a/packages/core/src/collab/index.ts +++ b/packages/core/src/collab/index.ts @@ -1,6 +1,12 @@ // Public API for @agent-native/core/collab // Storage +export { + CollabDocumentLifecycleError, + registerCollabLifecycle, + type CollabLifecyclePolicy, +} from "./lifecycle.js"; + export { loadYDocState, saveYDocState, diff --git a/packages/core/src/collab/lifecycle.spec.ts b/packages/core/src/collab/lifecycle.spec.ts new file mode 100644 index 00000000000..2982eae50bc --- /dev/null +++ b/packages/core/src/collab/lifecycle.spec.ts @@ -0,0 +1,170 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +import { createTestPglite } from "../a2a/test-pglite.js"; +import type { DbExec, DbExecStatement } from "../db/client.js"; +import { registerCollabLifecycle } from "./lifecycle.js"; +import { loadYDocRecord, saveYDocState, trySaveYDocState } from "./storage.js"; + +const runtime = vi.hoisted(() => ({ client: undefined as DbExec | undefined })); +vi.mock("../db/client.js", () => ({ + getDbExec: () => runtime.client, + isProductionServerlessFunctionRuntime: () => false, +})); + +describe("collaboration source lifecycle persistence", () => { + let db: Awaited>; + let unregister: (() => void) | undefined; + let beforeTransaction: (() => Promise) | undefined; + let failCommit = false; + + beforeEach(async () => { + db = await createTestPglite(); + await db.exec( + "CREATE TABLE documents (id TEXT PRIMARY KEY, trashed_at TEXT)", + ); + await db.exec( + "CREATE TABLE _collab_docs (doc_id TEXT PRIMARY KEY, yjs_state TEXT NOT NULL, text_snapshot TEXT NOT NULL, version INTEGER NOT NULL, updated_at TEXT NOT NULL)", + ); + await db + .prepare("INSERT INTO documents VALUES (?, NULL)") + .run("example-doc"); + const execute = async (query: DbExecStatement) => { + const sql = typeof query === "string" ? query : query.sql; + const args = typeof query === "string" ? [] : (query.args ?? []); + const result = await db.query(sql, args); + return { + rows: result.rows, + rowsAffected: result.affectedRows ?? result.rowCount ?? 0, + }; + }; + runtime.client = { + execute, + transaction: async (fn) => { + await beforeTransaction?.(); + return db.db.transaction(async (transaction) => { + const result = await fn({ + execute: async (query) => { + const sql = typeof query === "string" ? query : query.sql; + const args = typeof query === "string" ? [] : (query.args ?? []); + let index = 0; + const row = await transaction.query( + sql.replace(/\?/g, () => `$${++index}`), + args, + ); + return { rows: row.rows, rowsAffected: row.affectedRows ?? 0 }; + }, + }); + if (failCommit) throw new Error("example commit failure"); + return result; + }); + }, + }; + unregister = registerCollabLifecycle({ + table: "documents", + idColumn: "id", + deletedAtColumn: "trashed_at", + }); + }); + + afterEach(async () => { + unregister?.(); + beforeTransaction = undefined; + failCommit = false; + await db.close(); + }); + + it("rolls back collab persistence if the guarded transaction cannot commit", async () => { + failCommit = true; + await expect( + saveYDocState("example-doc", new Uint8Array([1]), "rejected"), + ).rejects.toThrow("example commit failure"); + expect(await loadYDocRecord("example-doc")).toBeNull(); + failCommit = false; + await saveYDocState("example-doc", new Uint8Array([2]), "accepted"); + expect((await loadYDocRecord("example-doc"))?.state).toEqual( + new Uint8Array([2]), + ); + }); + it.each(["save", "insert", "cas"])( + "rejects %s if trash commits after the caller's live read", + async (operation) => { + await saveYDocState("example-doc", new Uint8Array([1]), "initial"); + expect( + await db.prepare("SELECT trashed_at FROM documents").get(), + ).toEqual({ + trashed_at: null, + }); + beforeTransaction = async () => { + await db + .prepare("UPDATE documents SET trashed_at = 'example-trash'") + .run(); + }; + const write = + operation === "save" + ? saveYDocState("example-doc", new Uint8Array([2]), "rejected") + : trySaveYDocState( + "example-doc", + new Uint8Array([2]), + "rejected", + operation === "cas" ? 0 : null, + ); + await expect(write).rejects.toMatchObject({ + code: "DOCUMENT_TRASHED", + statusCode: 409, + }); + expect((await loadYDocRecord("example-doc"))?.state).toEqual( + new Uint8Array([1]), + ); + }, + ); + + it("cannot recreate collab state after permanent deletion", async () => { + await db.prepare("DELETE FROM documents").run(); + await expect( + saveYDocState("example-doc", new Uint8Array([1]), "late seed"), + ).rejects.toMatchObject({ + errorCode: "DOCUMENT_NOT_FOUND", + statusCode: 404, + }); + expect(await loadYDocRecord("example-doc")).toBeNull(); + }); + + it("guards the resolved source row and rejects unmapped IDs", async () => { + unregister?.(); + unregister = registerCollabLifecycle({ + table: "documents", + idColumn: "id", + deletedAtColumn: "trashed_at", + resolveSourceId: (id) => (id === "mapped-example" ? "example-doc" : null), + }); + await saveYDocState("mapped-example", new Uint8Array([1]), "saved"); + await db.prepare("UPDATE documents SET trashed_at = 'example-trash'").run(); + await expect( + saveYDocState("mapped-example", new Uint8Array([2]), "late"), + ).rejects.toMatchObject({ errorCode: "DOCUMENT_TRASHED" }); + await expect( + saveYDocState("unmapped-example", new Uint8Array([2]), "late"), + ).rejects.toMatchObject({ errorCode: "DOCUMENT_NOT_FOUND" }); + }); + + it("preserves unconfigured stores and rejects unsafe identifiers", async () => { + unregister?.(); + await saveYDocState("unscoped-example", new Uint8Array([1]), "generic app"); + expect(await loadYDocRecord("unscoped-example")).not.toBeNull(); + expect(() => + registerCollabLifecycle({ + table: "documents; DROP TABLE documents", + idColumn: "id", + deletedAtColumn: "trashed_at", + }), + ).toThrow(/SQL identifier/); + }); + + it("fails loudly when an adapter cannot run interactive transactions", async () => { + runtime.client = { execute: runtime.client!.execute }; + await expect( + saveYDocState("example-doc", new Uint8Array([1]), "rejected"), + ).rejects.toThrow(/interactive database transactions/); + expect(await loadYDocRecord("example-doc")).toBeNull(); + }); +}); diff --git a/packages/core/src/collab/lifecycle.ts b/packages/core/src/collab/lifecycle.ts new file mode 100644 index 00000000000..71d0bcf48a2 --- /dev/null +++ b/packages/core/src/collab/lifecycle.ts @@ -0,0 +1,95 @@ +import { getDbExec, type DbExec } from "../db/client.js"; + +export interface CollabLifecyclePolicy { + table: string; + idColumn: string; + deletedAtColumn: string; + resolveSourceId?: (docId: string) => string | null | Promise; +} + +export class CollabDocumentLifecycleError extends Error { + readonly code: "DOCUMENT_TRASHED" | "DOCUMENT_NOT_FOUND"; + readonly errorCode: "DOCUMENT_TRASHED" | "DOCUMENT_NOT_FOUND"; + readonly statusCode: number; + readonly data: { errorCode: string }; + + constructor(trashed: boolean) { + super(trashed ? "Document is in Trash." : "Document not found."); + this.name = "CollabDocumentLifecycleError"; + this.code = this.errorCode = trashed + ? "DOCUMENT_TRASHED" + : "DOCUMENT_NOT_FOUND"; + this.statusCode = trashed ? 409 : 404; + this.data = { errorCode: this.errorCode }; + } +} + +let lifecyclePolicy: CollabLifecyclePolicy | undefined; + +function identifier(value: string): string { + if (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(value)) { + throw new Error(`Invalid collaboration lifecycle SQL identifier: ${value}`); + } + return `"${value}"`; +} + +/** Register the source lifecycle for this process's shared collab store. */ +export function registerCollabLifecycle( + policy: CollabLifecyclePolicy, +): () => void { + identifier(policy.table); + identifier(policy.idColumn); + identifier(policy.deletedAtColumn); + if ( + lifecyclePolicy && + (lifecyclePolicy.table !== policy.table || + lifecyclePolicy.idColumn !== policy.idColumn || + lifecyclePolicy.deletedAtColumn !== policy.deletedAtColumn || + lifecyclePolicy.resolveSourceId !== policy.resolveSourceId) + ) { + throw new Error( + "The collaboration store already has a different lifecycle policy.", + ); + } + const registered = { ...policy }; + lifecyclePolicy = registered; + return () => { + if (lifecyclePolicy === registered) lifecyclePolicy = undefined; + }; +} + +export async function withCollabLifecycleWrite( + docId: string, + write: (tx: DbExec) => Promise, +): Promise { + const client = getDbExec(); + const policy = lifecyclePolicy; + if (!policy) return write(client); + const sourceId = policy.resolveSourceId + ? await policy.resolveSourceId(docId) + : docId; + if (!sourceId) throw new CollabDocumentLifecycleError(false); + if (!client.transaction) { + throw new Error( + "Collaboration lifecycle writes require interactive database transactions.", + ); + } + return client.transaction(async (tx) => { + const table = identifier(policy.table); + const id = identifier(policy.idColumn); + const deletedAt = identifier(policy.deletedAtColumn); + // The source lock and collab write must share a transaction so deletion + // cannot commit between the live check and the durable Yjs update. + const { rows } = await tx.execute({ + sql: `UPDATE ${table} SET ${id} = ${id} WHERE ${id} = ? RETURNING ${deletedAt} AS deleted_at`, + args: [sourceId], + }); + if (rows.length !== 1) throw new CollabDocumentLifecycleError(false); + if (!("deleted_at" in rows[0]) || rows[0].deleted_at === undefined) { + throw new Error("Document lifecycle state is unreadable."); + } + if (rows[0].deleted_at !== null) + throw new CollabDocumentLifecycleError(true); + return write(tx); + }); +} diff --git a/packages/core/src/collab/storage.ts b/packages/core/src/collab/storage.ts index feedef32936..6509ad90a09 100644 --- a/packages/core/src/collab/storage.ts +++ b/packages/core/src/collab/storage.ts @@ -7,6 +7,7 @@ import { getDbExec } from "../db/client.js"; import { ensureTableExists, ensureColumnExists } from "../db/ddl-guard.js"; +import { withCollabLifecycleWrite } from "./lifecycle.js"; let _initPromise: Promise | undefined; @@ -98,22 +99,23 @@ export async function trySaveYDocState( expectedVersion: number | null, ): Promise { await ensureTable(); - const client = getDbExec(); - const b64 = uint8ArrayToBase64(state); - const nowExpr = "NOW()::text"; - if (expectedVersion === null) { + return withCollabLifecycleWrite(docId, async (client) => { + const b64 = uint8ArrayToBase64(state); + const nowExpr = "NOW()::text"; + if (expectedVersion === null) { + const result = await client.execute({ + sql: `INSERT INTO _collab_docs (doc_id, yjs_state, text_snapshot, version, updated_at) VALUES (?, ?, ?, 0, ${nowExpr}) ON CONFLICT (doc_id) DO NOTHING`, + args: [docId, b64, textSnapshot], + }); + return result.rowsAffected > 0; + } + const result = await client.execute({ - sql: `INSERT INTO _collab_docs (doc_id, yjs_state, text_snapshot, version, updated_at) VALUES (?, ?, ?, 0, ${nowExpr}) ON CONFLICT (doc_id) DO NOTHING`, - args: [docId, b64, textSnapshot], + sql: `UPDATE _collab_docs SET yjs_state = ?, text_snapshot = ?, version = version + 1, updated_at = ${nowExpr} WHERE doc_id = ? AND version = ?`, + args: [b64, textSnapshot, docId, expectedVersion], }); return result.rowsAffected > 0; - } - - const result = await client.execute({ - sql: `UPDATE _collab_docs SET yjs_state = ?, text_snapshot = ?, version = version + 1, updated_at = ${nowExpr} WHERE doc_id = ? AND version = ?`, - args: [b64, textSnapshot, docId, expectedVersion], }); - return result.rowsAffected > 0; } /** Save Yjs state (Uint8Array) and a plain-text snapshot. */ @@ -123,24 +125,25 @@ export async function saveYDocState( textSnapshot: string, ): Promise { await ensureTable(); - const client = getDbExec(); - const b64 = uint8ArrayToBase64(state); - const nowExpr = "NOW()::text"; - const updated = await client.execute({ - sql: `UPDATE _collab_docs SET yjs_state = ?, text_snapshot = ?, version = version + 1, updated_at = ${nowExpr} WHERE doc_id = ?`, - args: [b64, textSnapshot, docId], - }); - if (updated.rowsAffected > 0) return; + return withCollabLifecycleWrite(docId, async (client) => { + const b64 = uint8ArrayToBase64(state); + const nowExpr = "NOW()::text"; + const updated = await client.execute({ + sql: `UPDATE _collab_docs SET yjs_state = ?, text_snapshot = ?, version = version + 1, updated_at = ${nowExpr} WHERE doc_id = ?`, + args: [b64, textSnapshot, docId], + }); + if (updated.rowsAffected > 0) return; - const inserted = await client.execute({ - sql: `INSERT INTO _collab_docs (doc_id, yjs_state, text_snapshot, version, updated_at) VALUES (?, ?, ?, 0, ${nowExpr}) ON CONFLICT (doc_id) DO NOTHING`, - args: [docId, b64, textSnapshot], - }); - if (inserted.rowsAffected > 0) return; + const inserted = await client.execute({ + sql: `INSERT INTO _collab_docs (doc_id, yjs_state, text_snapshot, version, updated_at) VALUES (?, ?, ?, 0, ${nowExpr}) ON CONFLICT (doc_id) DO NOTHING`, + args: [docId, b64, textSnapshot], + }); + if (inserted.rowsAffected > 0) return; - await client.execute({ - sql: `UPDATE _collab_docs SET yjs_state = ?, text_snapshot = ?, version = version + 1, updated_at = ${nowExpr} WHERE doc_id = ?`, - args: [b64, textSnapshot, docId], + await client.execute({ + sql: `UPDATE _collab_docs SET yjs_state = ?, text_snapshot = ?, version = version + 1, updated_at = ${nowExpr} WHERE doc_id = ?`, + args: [b64, textSnapshot, docId], + }); }); } diff --git a/packages/core/src/collab/ydoc-manager.spec.ts b/packages/core/src/collab/ydoc-manager.spec.ts index b7531ad2893..8a3bac70d9d 100644 --- a/packages/core/src/collab/ydoc-manager.spec.ts +++ b/packages/core/src/collab/ydoc-manager.spec.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as Y from "yjs"; const storageMocks = vi.hoisted(() => ({ loadYDocRecord: vi.fn(), @@ -47,4 +48,88 @@ describe("ydoc-manager", () => { expect(first).toBe(second); expect(storageMocks.loadYDocRecord).toHaveBeenCalledTimes(1); }); + + it.each(["update", "text", "search-replace", "json", "patch"])( + "discards a rejected %s mutation before a later successful write", + async (operation) => { + const initial = new Y.Doc(); + initial.getText("content").insert(0, "original"); + initial.getMap("data").set("title", "original"); + const paragraph = new Y.XmlElement("paragraph"); + const xmlText = new Y.XmlText(); + xmlText.insert(0, "original"); + paragraph.insert(0, [xmlText]); + initial.getXmlFragment("default").insert(0, [paragraph]); + let state = Y.encodeStateAsUpdate(initial); + let version = 0; + storageMocks.loadYDocRecord.mockImplementation(async () => ({ + state, + version, + })); + storageMocks.loadYDocVersion.mockImplementation(async () => version); + storageMocks.trySaveYDocState.mockRejectedValueOnce( + Object.assign(new Error("Document is in Trash."), { + code: "DOCUMENT_TRASHED", + statusCode: 409, + }), + ); + const manager = await import("./ydoc-manager.js"); + const { emitCollabUpdate } = await import("./emitter.js"); + vi.mocked(emitCollabUpdate).mockClear(); + let rejected: Promise; + if (operation === "update") { + const peer = new Y.Doc(); + Y.applyUpdate(peer, state); + peer.getText("content").insert(0, "rejected "); + rejected = manager.applyUpdate( + "example-doc", + Y.encodeStateAsUpdate(peer), + ); + peer.destroy(); + } else if (operation === "text") { + rejected = manager.applyText("example-doc", "rejected"); + } else if (operation === "search-replace") { + rejected = manager.searchAndReplace( + "example-doc", + "original", + "rejected", + ); + } else if (operation === "json") { + rejected = manager.applyJson("example-doc", { title: "rejected" }); + } else { + rejected = manager.applyPatchOps("example-doc", [ + { op: "set", path: "title", value: "rejected" }, + ]); + } + await expect(rejected).rejects.toMatchObject({ + code: "DOCUMENT_TRASHED", + }); + expect(emitCollabUpdate).not.toHaveBeenCalled(); + const restored = await manager.getDoc("example-doc"); + expect(restored.getText("content").toString()).toBe("original"); + expect(restored.getMap("data").get("title")).toBe("original"); + expect(restored.getXmlFragment("default").toString()).not.toContain( + "rejected", + ); + storageMocks.trySaveYDocState.mockImplementation( + async (_id, nextState) => { + state = nextState; + version++; + return true; + }, + ); + await manager.applyText("example-doc", "accepted"); + expect(emitCollabUpdate).toHaveBeenCalledTimes(1); + const persisted = new Y.Doc(); + Y.applyUpdate(persisted, state); + expect(persisted.getMap("data").get("title")).toBe("original"); + expect(persisted.getXmlFragment("default").toString()).not.toContain( + "rejected", + ); + expect(persisted.getText("content").toString()).toBe("accepted"); + manager.releaseDoc("example-doc"); + initial.destroy(); + persisted.destroy(); + }, + ); }); diff --git a/packages/core/src/collab/ydoc-manager.ts b/packages/core/src/collab/ydoc-manager.ts index a88c0b5047a..3b4ed751716 100644 --- a/packages/core/src/collab/ydoc-manager.ts +++ b/packages/core/src/collab/ydoc-manager.ts @@ -171,6 +171,11 @@ async function withDocWriteLock( await previous.catch(() => {}); try { return await fn(); + } catch (error) { + // A rejected mutation may already be present in the cached Y.Doc. + // Reload durable state before allowing the next writer to reuse it. + releaseDoc(docId); + throw error; } finally { release(); if (_writeLocks.get(docId) === chained) { @@ -514,24 +519,13 @@ export async function applyText( return snapshot; } - try { - await persistMergedState( - docId, - doc, - () => doc.getText(fieldName).toString(), - options.validateSnapshot, - validatedBaseVersion, - ); - } catch (error) { - // The rejected diff, and any cross-process state merged during the CAS - // read, now live only in this cached Y.Doc. Destroy it before throwing: - // neither the rejected update nor a compensating rollback should ever be - // persisted or emitted. Gating this on validateSnapshot left a pinned - // caller's rejected mutation cached forever, so the next successful - // write folded peer state on top of durably-rejected content. - releaseDoc(docId); - throw error; - } + await persistMergedState( + docId, + doc, + () => doc.getText(fieldName).toString(), + options.validateSnapshot, + validatedBaseVersion, + ); emitCollabUpdate(docId, uint8ArrayToBase64(update), requestSource); touchAgentPresence(docId, requestSource, { diff --git a/packages/core/src/server/collab-plugin.spec.ts b/packages/core/src/server/collab-plugin.spec.ts index 2a146fc0d84..6d626c2dd5a 100644 --- a/packages/core/src/server/collab-plugin.spec.ts +++ b/packages/core/src/server/collab-plugin.spec.ts @@ -1,15 +1,135 @@ +import { createApp } from "h3"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { + CollabDocumentLifecycleError, + registerCollabLifecycle, +} from "../collab/lifecycle.js"; +import * as manager from "../collab/ydoc-manager.js"; import { createCollabPlugin, normalizeCollabAccess, selectUnseededCollabRows, } from "./collab-plugin.js"; +vi.mock("../deploy/route-discovery.js", () => ({ + getMissingDefaultPlugins: vi.fn(async () => []), +})); +vi.mock("./auth.js", () => ({ + getSession: vi.fn(async () => ({ email: "editor@example.test" })), +})); +vi.mock("../org/context.js", () => ({ + getOrgContext: vi.fn(async () => null), +})); +vi.mock("../collab/emitter.js", () => ({ + getCollabEmitter: () => ({ on: vi.fn() }), +})); + afterEach(() => { vi.restoreAllMocks(); }); +describe("collab lifecycle HTTP responses", () => { + const mutations = [ + ["update", "applyUpdate", { update: "AAA=" }], + ["text", "applyText", { text: "changed" }], + ["search-replace", "searchAndReplace", { find: "old", replace: "new" }], + ["json", "applyJson", { json: { title: "changed" } }], + ["patch", "applyPatchOps", { ops: [] }], + ] as const; + + it.each(mutations)( + "serializes terminal errors from %s through the mounted framework route", + async (action, mutation, body) => { + const app = createApp(); + await createCollabPlugin({ + access: { mode: "all-authenticated" }, + autoSeed: false, + })({ h3: app }); + const persist = vi.spyOn(manager, mutation); + + for (const trashed of [true, false]) { + const error = new CollabDocumentLifecycleError(trashed); + persist.mockRejectedValueOnce(error); + const response = await app.request( + `http://example.test/_agent-native/collab/document-1/${action}`, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }, + ); + + expect(response.status).toBe(error.statusCode); + expect(await response.json()).toEqual({ + error: error.message, + errorCode: error.errorCode, + }); + } + expect(persist).toHaveBeenCalledTimes(2); + }, + ); + + it("leaves unexpected persistence failures to the framework error handler", async () => { + const app = createApp(); + await createCollabPlugin({ + access: { mode: "all-authenticated" }, + autoSeed: false, + })({ h3: app }); + vi.spyOn(manager, "applyUpdate").mockRejectedValueOnce( + new Error("Persistence unavailable"), + ); + const response = await app.request( + "http://example.test/_agent-native/collab/document-1/update", + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ update: "AAA=" }), + }, + ); + + expect(response.status).toBe(500); + expect(await response.json()).toEqual({ error: "Persistence unavailable" }); + }); +}); + +describe("collab lifecycle configuration", () => { + it("registers the configured source lifecycle when the plugin is created", () => { + createCollabPlugin({ + table: "example_documents", + idColumn: "document_id", + lifecycle: { deletedAtColumn: "trashed_at" }, + access: { mode: "all-authenticated" }, + }); + const unregister = registerCollabLifecycle({ + table: "example_documents", + idColumn: "document_id", + deletedAtColumn: "trashed_at", + }); + try { + expect(() => + registerCollabLifecycle({ + table: "other_documents", + idColumn: "id", + deletedAtColumn: "trashed_at", + }), + ).toThrow(/different lifecycle policy/); + } finally { + unregister(); + } + }); + + it("requires inverse source mapping for mapped collaboration IDs", () => { + expect(() => + createCollabPlugin({ + resolveCollabDocumentId: (id) => `example-${id}`, + lifecycle: { deletedAtColumn: "trashed_at" }, + access: { mode: "all-authenticated" }, + }), + ).toThrow(/requires resolveSourceId/); + }); +}); + describe("normalizeCollabAccess", () => { it("normalizes the resource access policy", () => { const resolveResourceId = (docId: string) => `parent-${docId}`; diff --git a/packages/core/src/server/collab-plugin.ts b/packages/core/src/server/collab-plugin.ts index 950aec65acf..05de40b7cbc 100644 --- a/packages/core/src/server/collab-plugin.ts +++ b/packages/core/src/server/collab-plugin.ts @@ -22,6 +22,11 @@ import { import { postAwareness, getActiveUsers } from "../collab/awareness.js"; import { getCollabEmitter } from "../collab/emitter.js"; +import { + CollabDocumentLifecycleError, + registerCollabLifecycle, + type CollabLifecyclePolicy, +} from "../collab/lifecycle.js"; import { getCollabState, postCollabUpdate, @@ -105,6 +110,11 @@ export interface CollabPluginOptions { contentColumn?: string; /** Column name for the document ID. Default: "id" */ idColumn?: string; + /** Reject persistence when the source row is missing or its deletion column is non-null. */ + lifecycle?: Pick< + CollabLifecyclePolicy, + "deletedAtColumn" | "resolveSourceId" + >; /** Whether to auto-seed existing documents on startup. Default: true */ autoSeed?: boolean; /** Map a source-table id to the id used by the collab document store. */ @@ -226,6 +236,14 @@ export function createCollabPlugin( } = options; const resolveCollabDocumentId = options.resolveCollabDocumentId ?? ((sourceId: string) => sourceId); + if (options.lifecycle) { + if (options.resolveCollabDocumentId && !options.lifecycle.resolveSourceId) { + throw new Error( + "Collaboration lifecycle requires resolveSourceId when document IDs are mapped.", + ); + } + registerCollabLifecycle({ table, idColumn, ...options.lifecycle }); + } const resourceType = normalizedAccess.mode === "resource" ? normalizedAccess.resourceType @@ -356,130 +374,136 @@ export function createCollabPlugin( const userEmail = session.email; const orgId = orgCtx?.orgId ?? undefined; - return runWithRequestContext({ userEmail, orgId }, async () => { - // Access check — require at least viewer for reads, editor for writes. - // Awareness routes (POST awareness / GET users) require the same - // level as other reads so that knowledge of who is editing a doc - // doesn't leak to users without access. - if (resourceType) { - const resourceId = resolveResourceId - ? await resolveResourceId(docId) - : docId; - if (!resourceId) { - setResponseStatus(event, 404); - return { error: "Not found" }; + try { + return await runWithRequestContext({ userEmail, orgId }, async () => { + // Access check — require at least viewer for reads, editor for writes. + // Awareness routes (POST awareness / GET users) require the same + // level as other reads so that knowledge of who is editing a doc + // doesn't leak to users without access. + if (resourceType) { + const resourceId = resolveResourceId + ? await resolveResourceId(docId) + : docId; + if (!resourceId) { + setResponseStatus(event, 404); + return { error: "Not found" }; + } + const isWrite = + (action === "update" && method === "POST") || + (action === "text" && method === "POST") || + (action === "search-replace" && method === "POST") || + (action === "json" && method === "POST") || + (action === "patch" && method === "POST"); + + if (isWrite) { + // assertAccess throws ForbiddenError (→ 403) if no editor access. + // Projected: only ownerEmail/orgId are read below, and a collab + // resource's body is the largest column it has — loading it here + // means every keystroke-driven update read the whole document + // twice, once for the ACL and once for the edit itself. + const access = await assertAccess( + resourceType, + resourceId, + "editor", + undefined, + { skipResourceBody: true }, + ); + const resource = access.resource; + const awarenessScope: CollabAwarenessScope = { + resourceType, + resourceId, + ...(typeof resource.ownerEmail === "string" + ? { owner: resource.ownerEmail } + : {}), + ...(typeof resource.orgId === "string" + ? { orgId: resource.orgId } + : {}), + }; + if (event.context) { + event.context._collabAwarenessScope = awarenessScope; + } + } else { + // resolveAccess returns null when no access; return 404 to avoid leaking existence. + // Projected: only ownerEmail/orgId are read below, and a collab + // resource's body is the largest column it has. + const access = await resolveAccess( + resourceType, + resourceId, + undefined, + { skipResourceBody: true }, + ); + if (!access) { + setResponseStatus(event, 404); + return { error: "Not found" }; + } + const resource = access.resource; + const awarenessScope: CollabAwarenessScope = { + resourceType, + resourceId, + ...(typeof resource.ownerEmail === "string" + ? { owner: resource.ownerEmail } + : {}), + ...(typeof resource.orgId === "string" + ? { orgId: resource.orgId } + : {}), + }; + if (event.context) { + event.context._collabAwarenessScope = awarenessScope; + } + } } - const isWrite = + + // Payload size limit for write operations + const isWriteAction = (action === "update" && method === "POST") || (action === "text" && method === "POST") || (action === "search-replace" && method === "POST") || (action === "json" && method === "POST") || (action === "patch" && method === "POST"); - if (isWrite) { - // assertAccess throws ForbiddenError (→ 403) if no editor access. - // Projected: only ownerEmail/orgId are read below, and a collab - // resource's body is the largest column it has — loading it here - // means every keystroke-driven update read the whole document - // twice, once for the ACL and once for the edit itself. - const access = await assertAccess( - resourceType, - resourceId, - "editor", - undefined, - { skipResourceBody: true }, - ); - const resource = access.resource; - const awarenessScope: CollabAwarenessScope = { - resourceType, - resourceId, - ...(typeof resource.ownerEmail === "string" - ? { owner: resource.ownerEmail } - : {}), - ...(typeof resource.orgId === "string" - ? { orgId: resource.orgId } - : {}), - }; - if (event.context) { - event.context._collabAwarenessScope = awarenessScope; - } - } else { - // resolveAccess returns null when no access; return 404 to avoid leaking existence. - // Projected: only ownerEmail/orgId are read below, and a collab - // resource's body is the largest column it has. - const access = await resolveAccess( - resourceType, - resourceId, - undefined, - { skipResourceBody: true }, + if (isWriteAction) { + const contentLength = Number( + event.headers?.get?.("content-length") ?? NaN, ); - if (!access) { - setResponseStatus(event, 404); - return { error: "Not found" }; + if (!isNaN(contentLength) && contentLength > maxPayloadBytes) { + setResponseStatus(event, 413); + return { + error: `Payload too large. Maximum is ${maxPayloadBytes} bytes.`, + }; } - const resource = access.resource; - const awarenessScope: CollabAwarenessScope = { - resourceType, - resourceId, - ...(typeof resource.ownerEmail === "string" - ? { owner: resource.ownerEmail } - : {}), - ...(typeof resource.orgId === "string" - ? { orgId: resource.orgId } - : {}), - }; + // Store limit in context so route handlers can enforce it on the + // parsed body when content-length is absent or spoofed. if (event.context) { - event.context._collabAwarenessScope = awarenessScope; + event.context._collabMaxPayloadBytes = maxPayloadBytes; } } - } - - // Payload size limit for write operations - const isWriteAction = - (action === "update" && method === "POST") || - (action === "text" && method === "POST") || - (action === "search-replace" && method === "POST") || - (action === "json" && method === "POST") || - (action === "patch" && method === "POST"); - - if (isWriteAction) { - const contentLength = Number( - event.headers?.get?.("content-length") ?? NaN, - ); - if (!isNaN(contentLength) && contentLength > maxPayloadBytes) { - setResponseStatus(event, 413); - return { - error: `Payload too large. Maximum is ${maxPayloadBytes} bytes.`, - }; - } - // Store limit in context so route handlers can enforce it on the - // parsed body when content-length is absent or spoofed. - if (event.context) { - event.context._collabMaxPayloadBytes = maxPayloadBytes; - } - } - if (action === "state" && method === "GET") - return getCollabState(event); - if (action === "update" && method === "POST") - return postCollabUpdate(event); - if (action === "text" && method === "POST") - return postCollabText(event); - if (action === "search-replace" && method === "POST") - return postCollabSearchReplace(event); - if (action === "json" && method === "POST") - return postCollabJson(event); - if (action === "json" && method === "GET") - return getCollabJson(event); - if (action === "patch" && method === "POST") - return postCollabPatch(event); - if (action === "awareness" && method === "POST") - return postAwareness(event); - if (action === "users" && method === "GET") - return getActiveUsers(event); - setResponseStatus(event, 404); - return { error: "Not found" }; - }); + if (action === "state" && method === "GET") + return getCollabState(event); + if (action === "update" && method === "POST") + return postCollabUpdate(event); + if (action === "text" && method === "POST") + return postCollabText(event); + if (action === "search-replace" && method === "POST") + return postCollabSearchReplace(event); + if (action === "json" && method === "POST") + return postCollabJson(event); + if (action === "json" && method === "GET") + return getCollabJson(event); + if (action === "patch" && method === "POST") + return postCollabPatch(event); + if (action === "awareness" && method === "POST") + return postAwareness(event); + if (action === "users" && method === "GET") + return getActiveUsers(event); + setResponseStatus(event, 404); + return { error: "Not found" }; + }); + } catch (error) { + if (!(error instanceof CollabDocumentLifecycleError)) throw error; + setResponseStatus(event, error.statusCode); + return { error: error.message, errorCode: error.errorCode }; + } }), ); diff --git a/templates/content/actions/_database-block-actions.ts b/templates/content/actions/_database-block-actions.ts index c078ef919d2..3ec16fa64b3 100644 --- a/templates/content/actions/_database-block-actions.ts +++ b/templates/content/actions/_database-block-actions.ts @@ -1,6 +1,6 @@ import { ActionContractError } from "@agent-native/core"; import { assertAccess } from "@agent-native/core/sharing"; -import { and, eq, isNull, sql } from "drizzle-orm"; +import { and, eq, isNull } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; @@ -51,6 +51,8 @@ import { type MutationContext, type RowSnapshot, } from "./_database-row-mutation.js"; +import { lockLiveDocuments } from "./_document-lifecycle.js"; +import { assertDocumentMutationAccess } from "./_document-mutation-access.js"; import { nanoid } from "./_property-utils.js"; type Db = ReturnType; @@ -708,26 +710,15 @@ export async function mutateDatabaseBlock( tx, input.target.rowDocumentId, ); - const [lockedDocument] = await tx - .update(schema.documents) - .set({ updatedAt: sql`${schema.documents.updatedAt}` }) - .where( - and( - eq(schema.documents.id, input.target.rowDocumentId), - isNull(schema.documents.trashedAt), - ), - ) - .returning({ id: schema.documents.id }); - if (!lockedDocument) { - contractError( - "ROW_NOT_FOUND", - "The exact database row was not found.", - { - documentId: input.target.rowDocumentId, - }, - 404, - ); - } + await lockLiveDocuments(tx, [ + input.target.databaseDocumentId, + input.target.rowDocumentId, + ]); + await assertDocumentMutationAccess( + tx, + [input.target.databaseDocumentId, input.target.rowDocumentId], + "editor", + ); const loaded = await loadField({ target: input.target, role: "editor", diff --git a/templates/content/actions/_document-edit-mutation.db.test.ts b/templates/content/actions/_document-edit-mutation.db.test.ts index 0add6054839..9e6d42a1eb3 100644 --- a/templates/content/actions/_document-edit-mutation.db.test.ts +++ b/templates/content/actions/_document-edit-mutation.db.test.ts @@ -2,6 +2,7 @@ import { rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { runWithRequestContext } from "@agent-native/core/server"; import { eq } from "drizzle-orm"; import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest"; @@ -20,8 +21,13 @@ let documentRevisionToken: typeof import("./_document-edit-mutation.js").documen beforeAll(async () => { process.env.DATABASE_URL = `pglite:${TEST_DB_PATH}`; ({ getDb, schema } = await import("../server/db/index.js")); - ({ mutateDocumentBody, documentRevisionToken } = - await import("./_document-edit-mutation.js")); + const mutations = await import("./_document-edit-mutation.js"); + documentRevisionToken = mutations.documentRevisionToken; + mutateDocumentBody = (args) => + runWithRequestContext( + { userEmail: args.ctx.userEmail, orgId: args.ctx.orgId }, + () => mutations.mutateDocumentBody(args), + ); const plugin = (await import("../server/plugins/db.js")).default; await plugin(undefined as never); }, 60_000); @@ -30,6 +36,7 @@ beforeEach(async () => { const db = getDb(); await db.delete(schema.documentEditReceipts); await db.delete(schema.documentVersions); + await db.delete(schema.documentShares); await db.delete(schema.documents); await db.insert(schema.documents).values({ id: DOCUMENT_ID, @@ -47,6 +54,31 @@ afterAll(() => { const ctx = { caller: "mcp" as const, userEmail: OWNER }; describe("revisioned document edit mutation", () => { + it("rejects a trashed page without creating history or an edit receipt", async () => { + await getDb() + .update(schema.documents) + .set({ trashedAt: new Date().toISOString() }) + .where(eq(schema.documents.id, DOCUMENT_ID)); + await expect( + mutateDocumentBody({ + documentId: DOCUMENT_ID, + baseRevision: documentRevisionToken(0, "alpha beta"), + idempotencyKey: "trashed-edit", + edits: [{ find: "alpha", replace: "late" }], + ctx, + }), + ).rejects.toMatchObject({ errorCode: "DOCUMENT_TRASHED" }); + expect(await getDb().select().from(schema.documentVersions)).toHaveLength( + 0, + ); + expect( + await getDb().select().from(schema.documentEditReceipts), + ).toHaveLength(0); + expect((await getDb().select().from(schema.documents))[0].content).toBe( + "alpha beta", + ); + }); + it("commits one revision/version/receipt and replays a double delivery", async () => { const input = { documentId: DOCUMENT_ID, @@ -139,6 +171,14 @@ describe("revisioned document edit mutation", () => { }); it("keeps idempotency receipts distinct for users in the same organization", async () => { + await getDb().insert(schema.documentShares).values({ + id: "another-editor-share", + resourceId: DOCUMENT_ID, + principalType: "user", + principalId: "another-editor@example.com", + role: "editor", + createdBy: OWNER, + }); const base = { documentId: DOCUMENT_ID, baseRevision: documentRevisionToken(0, "alpha beta"), diff --git a/templates/content/actions/_document-edit-mutation.ts b/templates/content/actions/_document-edit-mutation.ts index 981816a20a4..7f292c544f4 100644 --- a/templates/content/actions/_document-edit-mutation.ts +++ b/templates/content/actions/_document-edit-mutation.ts @@ -17,6 +17,8 @@ import { lockPrimaryBlocksFields, persistBlocksFieldIdentity, } from "./_blocks-field-identity.js"; +import { lockLiveDocuments } from "./_document-lifecycle.js"; +import { assertDocumentMutationAccess } from "./_document-mutation-access.js"; type Db = ReturnType; @@ -172,6 +174,12 @@ export async function mutateDocumentBody(args: { try { return await db.transaction(async (transaction) => { const tx = transaction as unknown as Db; + const primaryBlocksFields = await lockPrimaryBlocksFields( + tx, + args.documentId, + ); + await lockLiveDocuments(tx, [args.documentId]); + await assertDocumentMutationAccess(tx, [args.documentId], "editor"); const [stored] = await tx .select() .from(schema.documentEditReceipts) @@ -247,10 +255,6 @@ export async function mutateDocumentBody(args: { const now = nextDocumentUpdatedAt(document.updatedAt); const receiptId = crypto.randomUUID(); if (changed) { - const primaryBlocksFields = await lockPrimaryBlocksFields( - tx, - args.documentId, - ); const updated = await tx .update(schema.documents) .set({ diff --git a/templates/content/actions/_document-lifecycle.ts b/templates/content/actions/_document-lifecycle.ts new file mode 100644 index 00000000000..2f64f03dd38 --- /dev/null +++ b/templates/content/actions/_document-lifecycle.ts @@ -0,0 +1,42 @@ +import { ActionContractError } from "@agent-native/core"; +import { eq, sql } from "drizzle-orm"; + +import { getDb, schema } from "../server/db/index.js"; + +type Db = ReturnType; + +export function documentTrashedError() { + return new ActionContractError("This page is in Trash.", { + errorCode: "DOCUMENT_TRASHED", + statusCode: 409, + }); +} + +// Acquire database and membership locks first. Individual updates impose the +// same document lock order on PostgreSQL and SQLite, unlike an IN predicate. +export async function lockDocumentsForLifecycle(db: Db, documentIds: string[]) { + const documents = []; + for (const id of [...new Set(documentIds)].sort()) { + const [document] = await db + .update(schema.documents) + .set({ updatedAt: sql`${schema.documents.updatedAt}` }) + .where(eq(schema.documents.id, id)) + .returning(); + if (!document) { + throw new ActionContractError("Document not found.", { + errorCode: "DOCUMENT_NOT_FOUND", + statusCode: 404, + }); + } + documents.push(document); + } + return documents; +} + +export async function lockLiveDocuments(db: Db, documentIds: string[]) { + const documents = await lockDocumentsForLifecycle(db, documentIds); + if (documents.some((document) => document.trashedAt !== null)) { + throw documentTrashedError(); + } + return documents; +} diff --git a/templates/content/actions/_document-mutation-access.ts b/templates/content/actions/_document-mutation-access.ts new file mode 100644 index 00000000000..8cd1c038caf --- /dev/null +++ b/templates/content/actions/_document-mutation-access.ts @@ -0,0 +1,103 @@ +import { ActionContractError } from "@agent-native/core"; +import { getRequestUserEmail } from "@agent-native/core/server/request-context"; +import { accessFilter } from "@agent-native/core/sharing"; +import { and, asc, inArray, isNull, or, sql } from "drizzle-orm"; + +import { getDb, schema } from "../server/db/index.js"; +import { chunks } from "./_batch-utils.js"; +import { getContentOrganizationMembership } from "./_content-space-access.js"; + +// Call after locking documents. Holding existing grants prevents a concurrent +// revoke from racing the authorization check and the subsequent mutation. +export async function assertDocumentMutationAccess( + db: ReturnType, + documentIds: string[], + role: "viewer" | "editor" | "admin", +) { + const ids = [...new Set(documentIds)].sort(); + for (const batch of chunks(ids, 90)) { + await db + .select({ id: schema.documentShares.id }) + .from(schema.documentShares) + .where(inArray(schema.documentShares.resourceId, batch)) + .orderBy(asc(schema.documentShares.id)) + .for("share"); + let authorized = await db + .select({ id: schema.documents.id }) + .from(schema.documents) + .where( + and( + inArray(schema.documents.id, batch), + accessFilter( + schema.documents, + schema.documentShares, + undefined, + role, + { includePublic: role === "viewer" }, + ), + ), + ); + const userEmail = getRequestUserEmail(); + if (authorized.length !== batch.length && role === "viewer" && userEmail) { + const { orgMembers } = await import("@agent-native/core/org"); + const memberships = await db + .select({ orgId: orgMembers.orgId }) + .from(orgMembers) + .where( + and( + sql`LOWER(${orgMembers.email}) = ${userEmail.trim().toLowerCase()}`, + isNull(orgMembers.federationRemovalPendingAt), + ), + ) + .orderBy(asc(orgMembers.orgId)) + .for("share"); + const contexts = []; + for (const membership of memberships) { + if ( + await getContentOrganizationMembership(membership.orgId, userEmail, { + db, + }) + ) { + contexts.push({ userEmail, orgId: membership.orgId }); + } + } + if (contexts.length > 0) { + authorized = await db + .select({ id: schema.documents.id }) + .from(schema.documents) + .where( + and( + inArray(schema.documents.id, batch), + or( + accessFilter( + schema.documents, + schema.documentShares, + undefined, + role, + { includePublic: true }, + ), + ...contexts.map((context) => + accessFilter( + schema.documents, + schema.documentShares, + context, + role, + { includePublic: true }, + ), + ), + ), + ), + ); + } + } + if (authorized.length !== batch.length) { + throw new ActionContractError( + "You no longer have permission to change every page in this operation.", + { + errorCode: "DOCUMENT_MUTATION_ACCESS_CHANGED", + statusCode: 403, + }, + ); + } + } +} diff --git a/templates/content/actions/add-comment.test.ts b/templates/content/actions/add-comment.test.ts index 9c7bb334d55..e0fe599cae0 100644 --- a/templates/content/actions/add-comment.test.ts +++ b/templates/content/actions/add-comment.test.ts @@ -25,6 +25,9 @@ const mockAssertAccess = vi.hoisted(() => vi.mock("@agent-native/core/sharing", () => ({ assertAccess: (...args: unknown[]) => mockAssertAccess(...args), })); +vi.mock("./_document-lifecycle.js", () => ({ + lockLiveDocuments: vi.fn(async () => []), +})); vi.mock("@agent-native/core/server", () => ({ getRequestRunContext: () => ({ runId: "run-1" }), getRequestUserEmail: () => "author@example.com", diff --git a/templates/content/actions/add-comment.ts b/templates/content/actions/add-comment.ts index 1b5ccd7a43d..a3aa40f2481 100644 --- a/templates/content/actions/add-comment.ts +++ b/templates/content/actions/add-comment.ts @@ -10,6 +10,7 @@ import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; import { notifyDocumentComment } from "../server/lib/comment-notifications.js"; +import { lockLiveDocuments } from "./_document-lifecycle.js"; type Mention = { email: string; name: string }; @@ -150,6 +151,9 @@ export default defineAction({ }; const inserted = await db.transaction(async (tx) => { + await lockLiveDocuments(tx as unknown as ReturnType, [ + documentId, + ]); const existingReceipt = async () => { const [existing] = await tx .select() diff --git a/templates/content/actions/blocks-seeding.db.test.ts b/templates/content/actions/blocks-seeding.db.test.ts index 0a050197260..b724bee1ceb 100644 --- a/templates/content/actions/blocks-seeding.db.test.ts +++ b/templates/content/actions/blocks-seeding.db.test.ts @@ -127,6 +127,200 @@ async function blocksDefinitions(databaseId: string) { ); } +describe("property writes respect document lifecycle and existing grants", () => { + async function fixture(type: "text" | "blocks", primary = false) { + const database = await createDatabaseRow({ seeded: true }); + const db = getDb(); + const rowId = `lifecycle-row-${database.databaseId}`; + const propertyId = `lifecycle-property-${database.databaseId}`; + await db + .insert(schema.documents) + .values({ + id: rowId, + ownerEmail: OWNER, + title: "Example row", + content: "original", + }); + await db + .insert(schema.contentDatabaseItems) + .values({ + id: `lifecycle-item-${database.databaseId}`, + ownerEmail: OWNER, + databaseId: database.databaseId, + documentId: rowId, + }); + await db + .insert(schema.documentPropertyDefinitions) + .values({ + id: propertyId, + ownerEmail: OWNER, + databaseId: database.databaseId, + name: "Example field", + type, + optionsJson: JSON.stringify({ blocks: { primary } }), + }); + if (primary) + await db + .update(schema.contentDatabases) + .set({ primaryBlocksPropertyId: propertyId }) + .where(eq(schema.contentDatabases.id, database.databaseId)); + return { ...database, rowId, propertyId }; + } + + it.each(["scalar", "body", "additional"] as const)( + "rejects a pending %s property save after the row is trashed", + async (kind) => { + const target = await fixture( + kind === "scalar" ? "text" : "blocks", + kind === "body", + ); + const db = getDb(); + const originalTransaction = db.transaction.bind(db); + const transaction = vi + .spyOn(db, "transaction") + .mockImplementationOnce(async (callback: any, config?: any) => { + await db + .update(schema.documents) + .set({ trashedAt: new Date().toISOString() }) + .where(eq(schema.documents.id, target.rowId)); + return originalTransaction(callback, config); + }); + try { + await expect( + runWithRequestContext({ userEmail: OWNER }, () => + setDocumentPropertyAction.run({ + documentId: target.rowId, + databaseId: target.databaseId, + propertyId: target.propertyId, + value: "late value", + }), + ), + ).rejects.toMatchObject({ errorCode: "DOCUMENT_TRASHED" }); + } finally { + transaction.mockRestore(); + } + expect( + ( + await db + .select() + .from(schema.documents) + .where(eq(schema.documents.id, target.rowId)) + )[0].content, + ).toBe("original"); + expect( + await db + .select() + .from(schema.documentPropertyValues) + .where(eq(schema.documentPropertyValues.documentId, target.rowId)), + ).toHaveLength(0); + expect( + await db + .select() + .from(schema.documentBlockFieldContents) + .where( + eq(schema.documentBlockFieldContents.documentId, target.rowId), + ), + ).toHaveLength(0); + }, + ); + + it("preserves scalar edits granted by the database when the row is viewer-only", async () => { + const target = await fixture("text"); + const editor = "database-editor@example.com"; + const db = getDb(); + await db.insert(schema.documentShares).values([ + { + id: `database-edit-${target.databaseId}`, + resourceId: target.documentId, + principalType: "user", + principalId: editor, + role: "editor", + createdBy: OWNER, + }, + { + id: `row-view-${target.databaseId}`, + resourceId: target.rowId, + principalType: "user", + principalId: editor, + role: "viewer", + createdBy: OWNER, + }, + ]); + await runWithRequestContext({ userEmail: editor }, () => + setDocumentPropertyAction.run({ + documentId: target.rowId, + databaseId: target.databaseId, + propertyId: target.propertyId, + value: "allowed value", + }), + ); + expect( + ( + await db + .select() + .from(schema.documentPropertyValues) + .where(eq(schema.documentPropertyValues.documentId, target.rowId)) + )[0].valueJson, + ).toBe(JSON.stringify("allowed value")); + }); + + it("rejects a pending scalar edit after its database editor grant is revoked", async () => { + const target = await fixture("text"); + const editor = "revoked-database-editor@example.com"; + const db = getDb(); + const shareId = `database-revoke-${target.databaseId}`; + await db.insert(schema.documentShares).values([ + { + id: shareId, + resourceId: target.documentId, + principalType: "user", + principalId: editor, + role: "editor", + createdBy: OWNER, + }, + { + id: `row-revoke-view-${target.databaseId}`, + resourceId: target.rowId, + principalType: "user", + principalId: editor, + role: "viewer", + createdBy: OWNER, + }, + ]); + const originalTransaction = db.transaction.bind(db); + const transaction = vi + .spyOn(db, "transaction") + .mockImplementationOnce(async (callback: any, config?: any) => { + await db + .delete(schema.documentShares) + .where(eq(schema.documentShares.id, shareId)); + return originalTransaction(callback, config); + }); + try { + await expect( + runWithRequestContext({ userEmail: editor }, () => + setDocumentPropertyAction.run({ + documentId: target.rowId, + databaseId: target.databaseId, + propertyId: target.propertyId, + value: "late value", + }), + ), + ).rejects.toMatchObject({ + errorCode: "DOCUMENT_MUTATION_ACCESS_CHANGED", + }); + } finally { + transaction.mockRestore(); + } + expect( + await db + .select() + .from(schema.documentPropertyValues) + .where(eq(schema.documentPropertyValues.documentId, target.rowId)), + ).toHaveLength(0); + }); +}); + describe("seedDefaultBlocksField — single-primary invariant (findings 1, 2)", () => { it("revalidates space access through the database transaction", async () => { let transactionDb: any; diff --git a/templates/content/actions/comment-submission.db.test.ts b/templates/content/actions/comment-submission.db.test.ts index d2c5b2d9a9e..42c36df3f8d 100644 --- a/templates/content/actions/comment-submission.db.test.ts +++ b/templates/content/actions/comment-submission.db.test.ts @@ -44,6 +44,7 @@ let db: ReturnType; let schema: typeof import("../server/db/schema.js"); let add: typeof import("./add-comment.js").default; let update: typeof import("./update-comment.js").default; +let remove: typeof import("./delete-comment.js").default; beforeAll(async () => { process.env.DATABASE_URL = `pglite:${dbPath}`; const module = await import("../server/db/index.js"); @@ -52,6 +53,12 @@ beforeAll(async () => { await (await import("../server/plugins/db.js")).default(undefined as any); add = (await import("./add-comment.js")).default; update = (await import("./update-comment.js")).default; + remove = (await import("./delete-comment.js")).default; + await db.insert(schema.documents).values({ + id: "receipt-fixture", + ownerEmail: "owner@example.test", + title: "Comment fixture", + }); }, 60000); afterAll(() => rmSync(dbPath, { force: true, recursive: true })); const create = (args: Record) => @@ -64,6 +71,67 @@ const resolve = (id: string) => (update as any).run({ id, documentId: "receipt-fixture", resolved: true }); describe("comment receipts and thread state on PostgreSQL-compatible storage", () => { + it.each(["add", "reply", "edit", "resolve", "reopen", "delete"])( + "rejects %s when Trash commits after access check and before the mutation transaction", + async (operation) => { + const documentId = `trash-comment-${operation}`; + await db.insert(schema.documents).values({ + id: documentId, + ownerEmail: "owner@example.test", + title: "Comment lifecycle fixture", + }); + const root = await create({ documentId }); + if (operation === "reopen") { + await (update as any).run({ id: root.id, resolved: true }); + } + const before = await db + .select() + .from(schema.documentComments) + .where(eq(schema.documentComments.documentId, documentId)); + const { assertAccess } = await import("@agent-native/core/sharing"); + vi.mocked(assertAccess).mockImplementationOnce(async () => { + await db + .update(schema.documents) + .set({ + trashedAt: new Date().toISOString(), + trashRootId: documentId, + }) + .where(eq(schema.documents.id, documentId)); + return { + resource: { + ownerEmail: "owner@example.test", + title: "Fixture", + orgId: null, + }, + } as Awaited>; + }); + const attempt = + operation === "add" + ? create({ documentId, content: "Rejected new comment" }) + : operation === "reply" + ? create({ documentId, threadId: root.id, parentId: root.id }) + : operation === "delete" + ? (remove as any).run({ id: root.id, documentId }) + : (update as any).run({ + id: root.id, + documentId, + ...(operation === "edit" + ? { content: "Rejected edit" } + : { resolved: operation === "resolve" }), + }); + await expect(attempt).rejects.toMatchObject({ + errorCode: "DOCUMENT_TRASHED", + statusCode: 409, + }); + expect( + await db + .select() + .from(schema.documentComments) + .where(eq(schema.documentComments.documentId, documentId)), + ).toEqual(before); + }, + ); + it("deduplicates overlapping UUID submissions in the database", async () => { const clientOperationId = crypto.randomUUID(); const results = await Promise.all([ diff --git a/templates/content/actions/delete-comment.ts b/templates/content/actions/delete-comment.ts index 5cac23f9e67..2b5b05741bc 100644 --- a/templates/content/actions/delete-comment.ts +++ b/templates/content/actions/delete-comment.ts @@ -6,6 +6,7 @@ import { and, eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; +import { lockLiveDocuments } from "./_document-lifecycle.js"; export default defineAction({ description: @@ -39,14 +40,19 @@ export default defineAction({ await assertAccess("document", comment.documentId, "editor"); } - await db - .delete(schema.documentComments) - .where( - and( - eq(schema.documentComments.id, args.id), - eq(schema.documentComments.documentId, comment.documentId), - ), - ); + await db.transaction(async (tx) => { + await lockLiveDocuments(tx as unknown as ReturnType, [ + comment.documentId, + ]); + await tx + .delete(schema.documentComments) + .where( + and( + eq(schema.documentComments.id, args.id), + eq(schema.documentComments.documentId, comment.documentId), + ), + ); + }); await writeAppState("refresh-signal", { ts: Date.now() }); return { ok: true }; diff --git a/templates/content/actions/document-history.db.test.ts b/templates/content/actions/document-history.db.test.ts index 638bf678410..48fd1fff80c 100644 --- a/templates/content/actions/document-history.db.test.ts +++ b/templates/content/actions/document-history.db.test.ts @@ -59,6 +59,7 @@ beforeEach(async () => { writeAppStateMock.mockReset(); writeAppStateMock.mockResolvedValue(undefined); await getDb().delete(schema.documentVersions); + await getDb().delete(schema.documentShares); await getDb().delete(schema.documents); const now = new Date(Date.now() - 60_000).toISOString(); await getDb().insert(schema.documents).values({ @@ -104,6 +105,71 @@ function inlineDatabaseBlock(args: { } describe("grouped document history", () => { + it.each(["trash", "revoke"] as const)( + "rejects a history restore when %s commits after access was checked", + async (change) => { + const db = getDb(); + const editor = "pending-history-editor@example.com"; + await db.insert(schema.documentShares).values({ + id: `pending-history-share-${change}`, + resourceId: DOCUMENT_ID, + principalType: "user", + principalId: editor, + role: "editor", + createdBy: OWNER, + }); + await db.insert(schema.documentVersions).values({ + id: "pending-history-target", + ownerEmail: OWNER, + documentId: DOCUMENT_ID, + title: "Earlier", + content: "earlier body", + createdAt: new Date().toISOString(), + }); + const before = await currentDocument(); + const originalTransaction = db.transaction.bind(db); + const transaction = vi + .spyOn(db, "transaction") + .mockImplementationOnce(async (callback: any, config?: any) => { + if (change === "trash") + await db + .update(schema.documents) + .set({ trashedAt: new Date().toISOString() }) + .where(eq(schema.documents.id, DOCUMENT_ID)); + else + await db + .delete(schema.documentShares) + .where( + eq(schema.documentShares.id, `pending-history-share-${change}`), + ); + return originalTransaction(callback, config); + }); + try { + await expect( + runWithRequestContext({ userEmail: editor }, () => + restoreDocumentVersion.run({ + documentId: DOCUMENT_ID, + versionId: "pending-history-target", + expectedUpdatedAt: before.updatedAt, + }), + ), + ).rejects.toMatchObject({ + errorCode: + change === "trash" + ? "DOCUMENT_TRASHED" + : "DOCUMENT_MUTATION_ACCESS_CHANGED", + }); + } finally { + transaction.mockRestore(); + } + expect(await currentDocument()).toMatchObject({ + title: before.title, + content: before.content, + }); + expect(await db.select().from(schema.documentVersions)).toHaveLength(1); + }, + ); + it("retains every saved checkpoint in session A and attributes session B to its own result", async () => { let current = await currentDocument(); for (const content of ["session A first", "session A final"]) { diff --git a/templates/content/actions/edit-document.ts b/templates/content/actions/edit-document.ts index f79af6c93a9..6a0a9cfd92e 100644 --- a/templates/content/actions/edit-document.ts +++ b/templates/content/actions/edit-document.ts @@ -24,6 +24,11 @@ import { persistBlocksFieldIdentity, } from "./_blocks-field-identity.js"; import { mutateDocumentBody } from "./_document-edit-mutation.js"; +import { + documentTrashedError, + lockLiveDocuments, +} from "./_document-lifecycle.js"; +import { assertDocumentMutationAccess } from "./_document-mutation-access.js"; import { editLinkedLocalDocumentThroughBrowser } from "./_linked-local-document-edit.js"; interface TextEdit { @@ -237,6 +242,7 @@ export default defineAction({ const access = await assertAccess("document", id, "editor"); const existing = access.resource; + if (existing.trashedAt) throw documentTrashedError(); const isExternalCaller = ctx?.caller === "tool" || ctx?.caller === "mcp" || @@ -422,6 +428,8 @@ export default defineAction({ try { await db.transaction(async (tx: any) => { const primaryBlocksFields = await lockPrimaryBlocksFields(tx, id); + await lockLiveDocuments(tx, [id]); + await assertDocumentMutationAccess(tx, [id], "editor"); const mirrored = await tx .update(schema.documents) .set({ diff --git a/templates/content/actions/get-document.ts b/templates/content/actions/get-document.ts index 7e84eda5992..1d7b5f27c3e 100644 --- a/templates/content/actions/get-document.ts +++ b/templates/content/actions/get-document.ts @@ -4,6 +4,7 @@ import { getRequestUserEmail } from "@agent-native/core/server/request-context"; import { roleSatisfies } from "@agent-native/core/sharing"; import { z } from "zod"; +import { documentTrashedError } from "./_document-lifecycle.js"; import { getDb } from "../server/db/index.js"; import { parseDocumentHideFromSearch } from "../server/lib/documents.js"; import { favoriteDocumentIds } from "./_content-favorites.js"; @@ -85,9 +86,7 @@ export default defineAction({ access.resource.trashedAt || (await isSoftDeletedDatabaseDocument(args.id)) ) { - throw Object.assign(new Error(`Document "${args.id}" not found`), { - statusCode: 404, - }); + throw documentTrashedError(); } const doc = access.resource; if (args.databaseDocumentId && !args.databaseId) { diff --git a/templates/content/actions/restore-document-version.ts b/templates/content/actions/restore-document-version.ts index 614e963234e..0468279a491 100644 --- a/templates/content/actions/restore-document-version.ts +++ b/templates/content/actions/restore-document-version.ts @@ -2,7 +2,7 @@ import { ActionContractError } from "@agent-native/core"; import { defineAction } from "@agent-native/core/action"; import { writeAppState } from "@agent-native/core/application-state"; import { assertAccess } from "@agent-native/core/sharing"; -import { and, eq } from "drizzle-orm"; +import { and, eq, inArray, or } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; @@ -19,10 +19,13 @@ import { persistBlocksFieldIdentity, } from "./_blocks-field-identity.js"; import { reconcileInlineDatabasesForDocumentWithDb } from "./_content-database-lifecycle.js"; +import { lockContentDatabaseMutation } from "./_content-database-mutation-lock.js"; import { documentContentHash, documentRevisionToken, } from "./_document-edit-mutation.js"; +import { lockLiveDocuments } from "./_document-lifecycle.js"; +import { assertDocumentMutationAccess } from "./_document-mutation-access.js"; function isLinkedLocalSource( documentId: string, @@ -80,33 +83,50 @@ export default defineAction({ const db = getDb(); let softDeletedDatabaseIds: string[] = []; const updated = await db.transaction(async (rawTx) => { - const tx = rawTx as any; - await tx - .select({ id: schema.documents.id }) - .from(schema.documents) + const tx = rawTx as unknown as ReturnType; + const databases = await tx + .select({ + id: schema.contentDatabases.id, + spaceId: schema.contentDatabases.spaceId, + systemRole: schema.contentDatabases.systemRole, + }) + .from(schema.contentDatabases) .where( - and( - eq(schema.documents.id, documentId), - eq(schema.documents.ownerEmail, ownerEmail), + or( + eq(schema.contentDatabases.documentId, documentId), + eq(schema.contentDatabases.ownerDocumentId, documentId), ), - ) - .for("update"); - const [current] = await tx - .select() - .from(schema.documents) - .where( - and( - eq(schema.documents.id, documentId), - eq(schema.documents.ownerEmail, ownerEmail), - ), - ) - .limit(1); - if (!current) { - throw new ActionContractError("Document not found.", { - errorCode: "DOCUMENT_NOT_FOUND", - statusCode: 404, - }); + ); + for (const databaseId of databases + .map((database) => database.id) + .sort()) { + await lockContentDatabaseMutation(tx, databaseId); } + const spaceIds = databases.flatMap((database) => + database.systemRole === "files" && database.spaceId + ? [database.spaceId] + : [], + ); + const references = + spaceIds.length > 0 + ? await tx + .select({ + documentId: schema.contentSpaceCatalogItems.documentId, + }) + .from(schema.contentSpaceCatalogItems) + .where(inArray(schema.contentSpaceCatalogItems.spaceId, spaceIds)) + : []; + const primaryBlocksFields = await lockPrimaryBlocksFields(tx, documentId); + const lockedDocuments = await lockLiveDocuments(tx, [ + documentId, + ...references.map( + (reference: { documentId: string }) => reference.documentId, + ), + ]); + await assertDocumentMutationAccess(tx, [documentId], "editor"); + const current = lockedDocuments.find( + (document) => document.id === documentId, + )!; if (isLinkedLocalSource(documentId, current)) { linkedLocalRestoreUnsupported(); } @@ -147,10 +167,6 @@ export default defineAction({ return current; } const now = nextDocumentUpdatedAt(current.updatedAt); - const primaryBlocksFields = await lockPrimaryBlocksFields( - tx as unknown as ReturnType, - documentId, - ); const applied = await tx .update(schema.documents) .set({ diff --git a/templates/content/actions/set-document-property.ts b/templates/content/actions/set-document-property.ts index d358c061c1d..e429a535c9d 100644 --- a/templates/content/actions/set-document-property.ts +++ b/templates/content/actions/set-document-property.ts @@ -20,6 +20,8 @@ import { import { lockContentDatabaseMutation } from "./_content-database-mutation-lock.js"; import { resolveContentDocumentAccess } from "./_content-document-access.js"; import { lockDatabaseMemberships } from "./_database-membership-lock.js"; +import { lockLiveDocuments } from "./_document-lifecycle.js"; +import { assertDocumentMutationAccess } from "./_document-mutation-access.js"; import { getDatabaseById, listPropertiesForDatabaseDocuments, @@ -106,10 +108,23 @@ export default defineAction({ parsePropertyOptions(definition.optionsJson), ); await db.transaction(async (tx) => { + await lockContentDatabaseMutation( + tx as unknown as ReturnType, + database.id, + ); const primaryBlocksFields = await lockPrimaryBlocksFields( tx as unknown as ReturnType, documentId, ); + await lockLiveDocuments(tx as unknown as ReturnType, [ + database.documentId, + documentId, + ]); + await assertDocumentMutationAccess( + tx as unknown as ReturnType, + [database.documentId, documentId], + "editor", + ); const [lockedDefinition] = await tx .select() .from(schema.documentPropertyDefinitions) @@ -255,6 +270,20 @@ export default defineAction({ ); if (!lockedDatabase) throw new Error("Database is no longer active."); await lockDatabaseMemberships(tx, [membership.id]); + await lockLiveDocuments(tx as unknown as ReturnType, [ + database.documentId, + documentId, + ]); + await assertDocumentMutationAccess( + tx as unknown as ReturnType, + [database.documentId], + "editor", + ); + await assertDocumentMutationAccess( + tx as unknown as ReturnType, + [documentId], + "viewer", + ); const [lockedDefinition] = await tx .select() .from(schema.documentPropertyDefinitions) diff --git a/templates/content/actions/update-comment.test.ts b/templates/content/actions/update-comment.test.ts index f176db2d954..52d74930daa 100644 --- a/templates/content/actions/update-comment.test.ts +++ b/templates/content/actions/update-comment.test.ts @@ -23,6 +23,9 @@ const mockWriteAppState = vi.hoisted(() => vi.fn()); vi.mock("@agent-native/core/sharing", () => ({ assertAccess: (...args: unknown[]) => mockAssertAccess(...args), })); +vi.mock("./_document-lifecycle.js", () => ({ + lockLiveDocuments: vi.fn(async () => []), +})); vi.mock("@agent-native/core/server/request-context", () => ({ getRequestUserEmail: () => mockGetUserEmail(), diff --git a/templates/content/actions/update-comment.ts b/templates/content/actions/update-comment.ts index 188bfb21da3..887890cc606 100644 --- a/templates/content/actions/update-comment.ts +++ b/templates/content/actions/update-comment.ts @@ -6,6 +6,7 @@ import { and, eq } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; +import { lockLiveDocuments } from "./_document-lifecycle.js"; type Mention = { email: string; name: string }; @@ -106,6 +107,9 @@ export default defineAction({ if (args.resolved !== undefined) { await db.transaction(async (tx) => { + await lockLiveDocuments(tx as unknown as ReturnType, [ + comment.documentId, + ]); // Serialize replies and resolution before either takes its write snapshot. await tx .select({ id: schema.documentComments.id }) @@ -143,15 +147,20 @@ export default defineAction({ return { ok: true, resolved: args.resolved }; } - await db - .update(schema.documentComments) - .set(contentUpdates) - .where( - and( - eq(schema.documentComments.id, args.id), - eq(schema.documentComments.documentId, comment.documentId), - ), - ); + await db.transaction(async (tx) => { + await lockLiveDocuments(tx as unknown as ReturnType, [ + comment.documentId, + ]); + await tx + .update(schema.documentComments) + .set(contentUpdates) + .where( + and( + eq(schema.documentComments.id, args.id), + eq(schema.documentComments.documentId, comment.documentId), + ), + ); + }); await writeAppState("refresh-signal", { ts: Date.now() }); return { ok: true }; diff --git a/templates/content/actions/update-document.db.test.ts b/templates/content/actions/update-document.db.test.ts index ffb94a53f45..5c3b774bfdf 100644 --- a/templates/content/actions/update-document.db.test.ts +++ b/templates/content/actions/update-document.db.test.ts @@ -75,6 +75,120 @@ async function documentRow(documentId: string) { } describe("update-document compare-and-swap", () => { + it.each(["trash", "delete", "revoke"] as const)( + "rejects a pending body write when %s wins before the transaction", + async (change) => { + const documentId = await createDocument({ content: "original" }); + const db = getDb(); + const shareId = nextId("pending-editor-share"); + await db.insert(schema.documentShares).values({ + id: shareId, + resourceId: documentId, + principalType: "user", + principalId: EDITOR, + role: "editor", + createdBy: OWNER, + }); + const originalTransaction = db.transaction.bind(db); + const transaction = vi + .spyOn(db, "transaction") + .mockImplementationOnce(async (callback: any, config?: any) => { + if (change === "trash") + await db + .update(schema.documents) + .set({ trashedAt: new Date().toISOString() }) + .where(eq(schema.documents.id, documentId)); + else if (change === "delete") + await db + .delete(schema.documents) + .where(eq(schema.documents.id, documentId)); + else + await db + .delete(schema.documentShares) + .where(eq(schema.documentShares.id, shareId)); + return originalTransaction(callback, config); + }); + try { + await expect( + runWithRequestContext({ userEmail: EDITOR }, () => + updateDocumentAction.run({ id: documentId, content: "late body" }), + ), + ).rejects.toMatchObject({ + errorCode: + change === "trash" + ? "DOCUMENT_TRASHED" + : change === "delete" + ? "DOCUMENT_NOT_FOUND" + : "DOCUMENT_MUTATION_ACCESS_CHANGED", + }); + } finally { + transaction.mockRestore(); + } + expect((await documentRow(documentId))?.content).toBe( + change === "delete" ? undefined : "original", + ); + expect( + await db + .select() + .from(schema.documentVersions) + .where(eq(schema.documentVersions.documentId, documentId)), + ).toHaveLength(0); + }, + ); + + it("allows a viewer to favorite a known public page", async () => { + const documentId = await createDocument({ content: "public body" }); + await getDb() + .update(schema.documents) + .set({ visibility: "public" }) + .where(eq(schema.documents.id, documentId)); + const result = await runWithRequestContext({ userEmail: VIEWER }, () => + updateDocumentAction.run({ id: documentId, isFavorite: true }), + ); + expect(result).toMatchObject({ isFavorite: true, content: "public body" }); + }); + + it("preserves favorite access through another organization membership", async () => { + const documentId = await createDocument({ content: "organization body" }); + const { provisionContentSpaces } = await import("./_content-spaces.js"); + await runWithRequestContext({ userEmail: VIEWER }, () => + provisionContentSpaces(getDb(), VIEWER), + ); + const { organizations, orgMembers, ORG_MIGRATIONS } = + await import("@agent-native/core/org"); + const { runMigrations } = await import("@agent-native/core/db"); + await runMigrations(ORG_MIGRATIONS, { table: "example_org_migrations" })( + undefined as never, + ); + const orgId = nextId("example-organization"); + await getDb().insert(organizations).values({ + id: orgId, + name: "Example organization", + createdBy: OWNER, + createdAt: Date.now(), + }); + await getDb() + .insert(orgMembers) + .values({ + id: nextId("example-membership"), + orgId, + email: VIEWER, + role: "member", + joinedAt: Date.now(), + }); + await getDb() + .update(schema.documents) + .set({ visibility: "org", orgId }) + .where(eq(schema.documents.id, documentId)); + const result = await runWithRequestContext({ userEmail: VIEWER }, () => + updateDocumentAction.run({ id: documentId, isFavorite: true }), + ); + expect(result).toMatchObject({ + isFavorite: true, + content: "organization body", + }); + }); + it("rejects external full-body writes outside the revisioned edit protocol", async () => { const documentId = await createDocument({ content: "original" }); diff --git a/templates/content/actions/update-document.ts b/templates/content/actions/update-document.ts index 9235ca6639f..4a4131cdef9 100644 --- a/templates/content/actions/update-document.ts +++ b/templates/content/actions/update-document.ts @@ -11,7 +11,7 @@ import { validateGenerationCreativeContext, } from "@agent-native/creative-context/server"; import type { CreativeContextReuseLabel } from "@agent-native/creative-context/types"; -import { and, eq } from "drizzle-orm"; +import { and, eq, inArray } from "drizzle-orm"; import { z } from "zod"; import { getDb, schema } from "../server/db/index.js"; @@ -30,9 +30,11 @@ import { } from "./_blocks-field-identity.js"; import { BUILDER_CMS_BODY_CONTENT_KEY } from "./_builder-cms-source-adapter.js"; import { reconcileInlineDatabasesForDocument } from "./_content-database-lifecycle.js"; +import { lockContentDatabaseMutation } from "./_content-database-mutation-lock.js"; import { resolveContentDocumentAccess } from "./_content-document-access.js"; import { favoriteDocumentIds, + favoritesSystemIds, setFavoriteMembership, } from "./_content-favorites.js"; import { provisionContentSpaces } from "./_content-spaces.js"; @@ -40,6 +42,11 @@ import { documentContentHash, documentRevisionToken, } from "./_document-edit-mutation.js"; +import { + documentTrashedError, + lockLiveDocuments, +} from "./_document-lifecycle.js"; +import { assertDocumentMutationAccess } from "./_document-mutation-access.js"; import { serializeDocumentSource } from "./_document-source.js"; // Not (yet) part of the shared API surface — kept local to avoid touching @@ -415,6 +422,7 @@ export default defineAction({ : await assertAccess("document", id, "editor"); if (!access) throw new Error(`Document "${id}" not found`); const existing = access.resource; + if (existing.trashedAt) throw documentTrashedError(); const ownerEmail = existing.ownerEmail as string; const db = getDb(); @@ -533,11 +541,58 @@ export default defineAction({ let committedContentChanged = false; let committedContentBefore = existing.content; await db.transaction(async (tx) => { - await tx - .select({ id: schema.documents.id }) - .from(schema.documents) - .where(eq(schema.documents.id, id)) - .for("update"); + const transactionDb = tx as unknown as ReturnType; + const databases = await tx + .select({ + id: schema.contentDatabases.id, + spaceId: schema.contentDatabases.spaceId, + systemRole: schema.contentDatabases.systemRole, + }) + .from(schema.contentDatabases) + .where(eq(schema.contentDatabases.documentId, id)); + const databaseIds = new Set(databases.map((database) => database.id)); + if (favoriteChanged) + databaseIds.add( + favoritesSystemIds(requestUserEmail as string).databaseId, + ); + for (const databaseId of [...databaseIds].sort()) { + await lockContentDatabaseMutation(transactionDb, databaseId); + } + const renamedSpaceIds = + args.title !== undefined + ? databases.flatMap((database) => + database.systemRole === "files" && database.spaceId + ? [database.spaceId] + : [], + ) + : []; + const catalogReferences = + renamedSpaceIds.length > 0 + ? await tx + .select({ + documentId: schema.contentSpaceCatalogItems.documentId, + }) + .from(schema.contentSpaceCatalogItems) + .where( + inArray( + schema.contentSpaceCatalogItems.spaceId, + renamedSpaceIds, + ), + ) + : []; + const primaryBlocksFields = await lockPrimaryBlocksFields( + transactionDb, + id, + ); + await lockLiveDocuments(transactionDb, [ + id, + ...catalogReferences.map((reference) => reference.documentId), + ]); + await assertDocumentMutationAccess( + transactionDb, + [id], + favoriteOnly ? "viewer" : "editor", + ); const [historyBefore] = await tx .select({ title: schema.documents.title, @@ -574,12 +629,6 @@ export default defineAction({ updates.bodyRevision = historyBefore.bodyRevision + 1; } if (lockedIconChanged) updates.icon = args.icon; - const primaryBlocksFields = lockedContentChanged - ? await lockPrimaryBlocksFields( - tx as unknown as ReturnType, - id, - ) - : []; if (useContentCas) { const applied = await tx .update(schema.documents) diff --git a/templates/content/app/components/editor/DocumentEditor.layout.test.ts b/templates/content/app/components/editor/DocumentEditor.layout.test.ts index 5a80ed6d29a..0daf37bd624 100644 --- a/templates/content/app/components/editor/DocumentEditor.layout.test.ts +++ b/templates/content/app/components/editor/DocumentEditor.layout.test.ts @@ -198,7 +198,7 @@ describe("document editor layout", () => { const toolbar = readFileSync( new URL("./DocumentToolbar.tsx", import.meta.url), "utf8", - ); + ).replace(/\r\n/g, "\n"); expect(source).toContain('localSourceAccess === "available"'); expect(source).toContain("data-local-source-read-only"); expect(source).toContain('device: "Agent-Native Desktop"'); @@ -377,6 +377,75 @@ describe("document editor layout", () => { ).toEqual({ view: "error", admittedDocumentId: null }); }); + it("releases a failed load only after an authoritative fetch succeeds", () => { + const previous = { + documentId: "document-a", + baselineErrorUpdateCount: 1, + authoritativeFetchCount: 0, + failed: true, + }; + const input = { + previous, + documentId: "document-a", + admitted: false, + dataUpdatedAt: 200, + errorUpdateCount: 1, + errorUpdatedAt: 100, + isError: false, + }; + expect(updateDocumentLoadFailureState(input).failed).toBe(true); + expect( + updateDocumentLoadFailureState({ ...input, authoritativeFetchCount: 2 }) + .failed, + ).toBe(false); + }); + + it("removes an admitted editor when a background fetch reports Trash", () => { + expect( + documentEditorLoadState({ + documentId: "document-a", + admittedDocumentId: "document-a", + hasDocument: true, + isDocumentCreationPending: false, + isFetchedAfterMount: true, + isFetching: false, + isError: true, + hasLoadFailure: false, + isManualRetrying: false, + error: { status: 409, errorCode: "DOCUMENT_TRASHED" }, + }), + ).toEqual({ view: "unavailable", admittedDocumentId: null }); + }); + + it("does not use an older success to clear a newer failed fetch", () => { + const input = { + documentId: "document-a", + admitted: false, + dataUpdatedAt: 100, + errorUpdateCount: 2, + errorUpdatedAt: 200, + authoritativeFetchCount: 1, + }; + const failed = updateDocumentLoadFailureState({ + ...input, + previous: { + documentId: "document-a", + baselineErrorUpdateCount: 1, + authoritativeFetchCount: 0, + failed: true, + }, + isError: true, + }); + expect( + updateDocumentLoadFailureState({ + ...input, + previous: failed, + dataUpdatedAt: 300, + isError: false, + }).failed, + ).toBe(true); + }); + it("waits for manual Retry to finish before admitting its success", () => { expect( documentEditorLoadState({ diff --git a/templates/content/app/components/editor/DocumentEditor.tsx b/templates/content/app/components/editor/DocumentEditor.tsx index b111c3d4615..ad88ea36099 100644 --- a/templates/content/app/components/editor/DocumentEditor.tsx +++ b/templates/content/app/components/editor/DocumentEditor.tsx @@ -366,7 +366,13 @@ export function pageEditorSessionKey({ return `${documentId}:${databaseId ?? ""}:${databaseDocumentId ?? ""}`; } -export function PageEditorSurface({ +export function PageEditorSurface(props: PageEditorSurfaceProps) { + return ( + + ); +} + +function PageEditorSurfaceContent({ documentId, databaseId, databaseDocumentId, @@ -389,6 +395,7 @@ export function PageEditorSurface({ isError, isFetchedAfterMount, isFetching, + authoritativeFetchCount, } = documentQuery; const navigate = useNavigate(); const queryClient = useQueryClient(); @@ -397,6 +404,22 @@ export function PageEditorSurface({ >(null); const admittedDocumentIdRef = useRef(null); const loadFailureRef = useRef(null); + const unavailableErrorRef = useRef<{ + error: unknown; + fetchCount: number; + } | null>(null); + if (isError && isDocumentLoadUnavailableError(error)) { + unavailableErrorRef.current = { + error, + fetchCount: authoritativeFetchCount, + }; + } else if ( + !isError && + unavailableErrorRef.current && + authoritativeFetchCount > unavailableErrorRef.current.fetchCount + ) { + unavailableErrorRef.current = null; + } const document = queriedDocument?.id === documentId ? queriedDocument : undefined; const loadFailure = updateDocumentLoadFailureState({ @@ -407,6 +430,7 @@ export function PageEditorSurface({ errorUpdateCount, errorUpdatedAt, isError, + authoritativeFetchCount, }); loadFailureRef.current = loadFailure; const loadState = documentEditorLoadState({ @@ -418,10 +442,10 @@ export function PageEditorSurface({ : false, isFetchedAfterMount, isFetching, - isError, + isError: isError || unavailableErrorRef.current !== null, hasLoadFailure: loadFailure.failed, isManualRetrying: manualRetryDocumentId === documentId, - error, + error: unavailableErrorRef.current?.error ?? error, }); admittedDocumentIdRef.current = loadState.admittedDocumentId; @@ -438,6 +462,7 @@ export function PageEditorSurface({ loadFailureRef.current = { documentId, baselineErrorUpdateCount: errorUpdateCount, + authoritativeFetchCount, failed: false, }; await documentQuery.refetch(); @@ -558,6 +583,14 @@ export function documentEditorLoadState({ const activeAdmittedDocumentId = admittedDocumentId === documentId ? admittedDocumentId : null; + if ( + isError && + isDocumentLoadUnavailableError(error) && + !isDocumentCreationPending + ) { + return { view: "unavailable" as const, admittedDocumentId: null }; + } + if ( hasDocument && (isDocumentCreationPending || activeAdmittedDocumentId === documentId) @@ -592,6 +625,7 @@ type DocumentLoadFailureState = { documentId: string; baselineErrorUpdateCount: number; failed: boolean; + authoritativeFetchCount?: number; }; export function updateDocumentLoadFailureState({ @@ -602,6 +636,7 @@ export function updateDocumentLoadFailureState({ errorUpdateCount, errorUpdatedAt, isError, + authoritativeFetchCount = 0, }: { previous: DocumentLoadFailureState | null; documentId: string; @@ -610,15 +645,25 @@ export function updateDocumentLoadFailureState({ errorUpdateCount: number; errorUpdatedAt: number; isError: boolean; + authoritativeFetchCount?: number; }): DocumentLoadFailureState { if (previous?.documentId !== documentId) { return { documentId, baselineErrorUpdateCount: errorUpdateCount, + authoritativeFetchCount, failed: isError || (errorUpdateCount > 0 && errorUpdatedAt > dataUpdatedAt), }; } + if (authoritativeFetchCount > (previous.authoritativeFetchCount ?? 0)) { + return { + documentId, + baselineErrorUpdateCount: errorUpdateCount, + authoritativeFetchCount, + failed: isError, + }; + } if (admitted || previous.failed) return previous; return errorUpdateCount > previous.baselineErrorUpdateCount ? { ...previous, failed: true } @@ -630,7 +675,17 @@ export function isDocumentLoadUnavailableError(error: unknown) { error && typeof error === "object" ? (error as { status?: unknown }).status : undefined; - return status === 403 || status === 404; + return status === 403 || status === 404 || isDocumentTrashedError(error); +} + +export function isDocumentTrashedError(error: unknown) { + const errorCode = + error && typeof error === "object" + ? ((error as { errorCode?: unknown; data?: { errorCode?: unknown } }) + .errorCode ?? + (error as { data?: { errorCode?: unknown } }).data?.errorCode) + : undefined; + return errorCode === "DOCUMENT_TRASHED"; } export function resolveAcknowledgedDocumentSnapshot< @@ -1621,6 +1676,11 @@ function PageEditorSessionBody({ : null; const collabInitializationFailed = collabEnabled && collabInitialization.status === "error"; + useEffect(() => { + if (isDocumentTrashedError(collabInitialization)) { + void queryClient.invalidateQueries(documentQueryFilter(documentId)); + } + }, [collabInitialization, documentId, queryClient]); const editorCanEdit = canEdit && !bodyHydrationPending && diff --git a/templates/content/app/components/editor/DocumentToolbar.tsx b/templates/content/app/components/editor/DocumentToolbar.tsx index 72b78445386..59eb5500971 100644 --- a/templates/content/app/components/editor/DocumentToolbar.tsx +++ b/templates/content/app/components/editor/DocumentToolbar.tsx @@ -607,6 +607,7 @@ export function DocumentToolbar({ const [historyOpen, setHistoryOpen] = useState(false); const [databaseExportOpen, setDatabaseExportOpen] = useState(false); const [deleteDialogOpen, setDeleteDialogOpen] = useState(false); + const pageActionsTriggerRef = useRef(null); const [searchQuery, setSearchQuery] = useState(""); const [debouncedQuery, setDebouncedQuery] = useState(""); const [linkingPageId, setLinkingPageId] = useState(null); @@ -1080,6 +1081,7 @@ export function DocumentToolbar({ utilityPanel === "info" && "bg-accent text-foreground", )} aria-label={t("editor.toolbar.morePageActions")} + ref={pageActionsTriggerRef} > @@ -1598,7 +1600,12 @@ export function DocumentToolbar({ - + { + event.preventDefault(); + pageActionsTriggerRef.current?.focus(); + }} + > {t("sidebar.deletePageQuestion")} diff --git a/templates/content/app/components/editor/PageDraftRecovery.test.tsx b/templates/content/app/components/editor/PageDraftRecovery.test.tsx index 63aba0b43c9..df4b956be5a 100644 --- a/templates/content/app/components/editor/PageDraftRecovery.test.tsx +++ b/templates/content/app/components/editor/PageDraftRecovery.test.tsx @@ -77,7 +77,7 @@ describe("Page draft recovery", () => { container.remove(); }); it("keeps a conflicting restoration visible and never deletes its draft", async () => { - state.update.mockResolvedValue({ conflict: true }); + state.update.mockResolvedValue({ conflict: true, document: page }); act(render); await act(async () => container.querySelector("button")!.click(), @@ -93,6 +93,66 @@ describe("Page draft recovery", () => { expect(container.querySelector('[role="alert"]')).not.toBeNull(); expect(container.textContent).toContain("Draft body"); expect(container.querySelector("textarea")).toBeNull(); + expect(container.textContent).toContain("Saved body"); + expect(container.textContent).toContain("editor.keepLocalDraft"); + }); + it("restores after Trash only when the user chooses the draft over the displayed saved version", async () => { + state.update.mockResolvedValueOnce({ + conflict: true, + document: { ...page, updatedAt: "restored-version" }, + }); + act(render); + const restore = () => + container.querySelector("button")!.click(); + await act(async () => restore()); + expect(state.update).toHaveBeenCalledTimes(1); + expect(state.remove).not.toHaveBeenCalled(); + await act(async () => restore()); + expect(state.update).toHaveBeenLastCalledWith( + expect.objectContaining({ + baseUpdatedAt: "restored-version", + loadedUpdatedAt: "restored-version", + loadedContentWasEmpty: false, + }), + ); + expect(state.remove).toHaveBeenCalledTimes(1); + }); + it("requires another review when the saved page changes during the explicit choice", async () => { + state.update + .mockResolvedValueOnce({ conflict: true, document: page }) + .mockResolvedValueOnce({ + conflict: true, + document: { ...page, content: "Newer body", updatedAt: "v2" }, + }); + act(render); + const restore = () => + container.querySelector("button")!.click(); + await act(async () => restore()); + await act(async () => restore()); + expect(state.update).toHaveBeenCalledTimes(2); + expect(state.update).toHaveBeenLastCalledWith( + expect.objectContaining({ baseUpdatedAt: "v1" }), + ); + expect(state.remove).not.toHaveBeenCalled(); + expect(container.textContent).toContain("Newer body"); + expect(container.querySelector("textarea")).toBeNull(); + }); + it("does not transfer a reviewed choice to a newer private draft", async () => { + state.update.mockResolvedValue({ conflict: true, document: page }); + act(render); + await act(async () => + container.querySelector("button")!.click(), + ); + state.draft = { ...state.draft!, version: 4, content: "New draft" }; + act(render); + expect(container.textContent).not.toContain("editor.keepLocalDraft"); + await act(async () => + container.querySelector("button")!.click(), + ); + expect(state.update).toHaveBeenLastCalledWith( + expect.objectContaining({ baseUpdatedAt: "original-version" }), + ); + expect(state.remove).not.toHaveBeenCalled(); }); it("retains a draft with an unknown original version without overwriting the Page", async () => { state.draft!.baseDocumentUpdatedAt = null; diff --git a/templates/content/app/components/editor/PageDraftRecovery.tsx b/templates/content/app/components/editor/PageDraftRecovery.tsx index 5cf7747e35c..a4143f89147 100644 --- a/templates/content/app/components/editor/PageDraftRecovery.tsx +++ b/templates/content/app/components/editor/PageDraftRecovery.tsx @@ -37,6 +37,19 @@ export function PageDraftRecovery({ const [busy, setBusy] = useState(false); const [failed, setFailed] = useState(false); const draft = drafts.data?.draft; + const [conflict, setConflict] = useState<{ + document: Document; + draftVersion: number; + draftTitle: string; + draftContent: string; + } | null>(null); + const reviewedConflict = + conflict && + conflict.draftVersion === draft?.version && + conflict.draftTitle === draft.title && + conflict.draftContent === draft.content + ? conflict.document + : null; async function settleDraft(restore: boolean) { if (!draft || busy) return; @@ -47,22 +60,31 @@ export function PageDraftRecovery({ restore && (draft.title !== document.title || draft.content !== document.content) ) { - if (!draft.baseDocumentUpdatedAt) { + const baseUpdatedAt = + reviewedConflict?.updatedAt ?? draft.baseDocumentUpdatedAt; + if (!baseUpdatedAt) { throw new Error("The draft has no original document version."); } const saved = await update.mutateAsync({ id: document.id, title: draft.title, content: draft.content, - baseUpdatedAt: draft.baseDocumentUpdatedAt, - loadedUpdatedAt: draft.baseDocumentUpdatedAt, - loadedContentWasEmpty: draft.loadedContentWasEmpty === 1, + baseUpdatedAt, + loadedUpdatedAt: baseUpdatedAt, + loadedContentWasEmpty: reviewedConflict + ? reviewedConflict.content.length === 0 + : draft.loadedContentWasEmpty === 1, }); - if ( - isDocumentUpdateConflict(saved) || - saved.content !== draft.content || - saved.title !== draft.title - ) { + if (isDocumentUpdateConflict(saved)) { + setConflict({ + document: saved.document, + draftVersion: draft.version, + draftTitle: draft.title, + draftContent: draft.content, + }); + return; + } + if (saved.content !== draft.content || saved.title !== draft.title) { throw new Error("Draft restoration was not confirmed."); } } @@ -108,6 +130,22 @@ export function PageDraftRecovery({ {draft.content} + {reviewedConflict ? ( + <> +

+ {t("editor.toolbar.conflict")} +

+
+

+ {t("editor.savedPageRecovery")} +

+

{reviewedConflict.title}

+ 
+              {reviewedConflict.content}
+
+
+ + ) : null} {failed ? (

{t("empty.genericError")} @@ -119,7 +157,11 @@ export function PageDraftRecovery({ disabled={busy || documentBodyHydrationIsPending(document)} onClick={() => void settleDraft(true)} > - {t("editor.restorePreviewDraft")} + {t( + reviewedConflict + ? "editor.keepLocalDraft" + : "editor.restorePreviewDraft", + )}