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/.changeset/quiet-share-revocations.md b/.changeset/quiet-share-revocations.md new file mode 100644 index 00000000000..00238a03578 --- /dev/null +++ b/.changeset/quiet-share-revocations.md @@ -0,0 +1,7 @@ +--- +"@agent-native/core": patch +--- + +Notify the affected user when an authorized action removes an explicit user share, so their open views can refresh even when they use another organization. The recipient event contains only the generic action name, without resource details. + +`unshare-resource` preserves `ok: true` for an acknowledged revoke and adds `recipientInvalidation.status`: `recorded` confirms the refresh marker was written (not that a browser received it), `unconfirmed` reports a notification error without treating the committed revoke as failed, and `not_applicable` means no explicit user grant was removed. Unconfirmed delivery is logged and has no automatic durable retry. Group, organization, public-access, and general authorization behavior are unchanged. diff --git a/packages/core/docs/content/locales/ar-SA/sharing.mdx b/packages/core/docs/content/locales/ar-SA/sharing.mdx index 3c1e7c12bbc..76fa2347fde 100644 --- a/packages/core/docs/content/locales/ar-SA/sharing.mdx +++ b/packages/core/docs/content/locales/ar-SA/sharing.mdx @@ -147,6 +147,8 @@ import { ShareButton } from "@agent-native/core/client/sharing"; أخبر الوكيل "بمشاركة هذا التصميم مع فريق التسويق كمحررين" وسيقوم باستدعاء `share-resource` مقابل نفس نقطة النهاية التي يستخدمها UI. تظهر النتيجة في مربع حوار المشاركة في العرض التالي. +يعيد `unshare-resource` القيمة `ok: true` بعد نجاح إلغاء الوصول. عند إزالة إذن مباشر لمستخدم، تكون `recipientInvalidation.status` هي `recorded` إذا سُجّلت علامة التحديث العامة للمستلم، أو `unconfirmed` إذا تعذّر تأكيد التسجيل؛ ولا تضمن أي منهما التسليم. تعني `not_applicable` أنه لم يُزل إذن مباشر لمستخدم. عدم تأكيد الإشعار لا يتراجع عن إلغاء الوصول. لا تتضمن العلامة معرّف المورد أو عنوانه أو محتواه. + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/de-DE/sharing.mdx b/packages/core/docs/content/locales/de-DE/sharing.mdx index 8b4d29e4e1d..6ea4be8dfa7 100644 --- a/packages/core/docs/content/locales/de-DE/sharing.mdx +++ b/packages/core/docs/content/locales/de-DE/sharing.mdx @@ -147,6 +147,8 @@ Das Framework mountet diese actions automatisch in jeder Vorlage – der Agent n Sagen Sie dem Agenten: „Teilen Sie dieses Design mit dem Marketingteam als Redakteuren“, und er ruft `share-resource` für denselben Endpunkt auf, den UI verwendet. Das Ergebnis wird beim nächsten Rendern im Freigabedialog angezeigt. +`unshare-resource` gibt nach erfolgreichem Widerruf `ok: true` zurück. Bei einer entfernten direkten Benutzerfreigabe ist `recipientInvalidation.status` gleich `recorded`, wenn das allgemeine Aktualisierungssignal für den Empfänger gespeichert wurde, oder `unconfirmed`, wenn dies nicht bestätigt werden kann; beides garantiert keine Zustellung. `not_applicable` bedeutet, dass keine direkte Benutzerfreigabe entfernt wurde. Eine unbestätigte Benachrichtigung macht den Widerruf nicht rückgängig. Das Signal enthält weder Ressourcen-ID noch Titel oder Inhalt. + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/es-ES/sharing.mdx b/packages/core/docs/content/locales/es-ES/sharing.mdx index 6f109512a16..99f6d34d58d 100644 --- a/packages/core/docs/content/locales/es-ES/sharing.mdx +++ b/packages/core/docs/content/locales/es-ES/sharing.mdx @@ -147,6 +147,8 @@ El marco monta automáticamente estos actions en cada plantilla: el agente los l Dígale al agente "comparta este diseño con el equipo de marketing como editores" y llamará a `share-resource` contra el mismo punto final que utiliza UI. El resultado aparece en el cuadro de diálogo para compartir en el siguiente renderizado. +`unshare-resource` devuelve `ok: true` tras una revocación correcta. Para un permiso directo de usuario eliminado, `recipientInvalidation.status` es `recorded` si se registra la señal genérica de actualización del destinatario, o `unconfirmed` si no puede confirmarse el registro; ninguno garantiza la entrega. `not_applicable` significa que no se eliminó ningún permiso directo de usuario. La incertidumbre de la notificación no deshace la revocación. La señal no contiene el ID, el título ni el contenido del recurso. + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/fr-FR/sharing.mdx b/packages/core/docs/content/locales/fr-FR/sharing.mdx index 3c9e87f9a1b..f52a9b0309a 100644 --- a/packages/core/docs/content/locales/fr-FR/sharing.mdx +++ b/packages/core/docs/content/locales/fr-FR/sharing.mdx @@ -147,6 +147,8 @@ Le framework monte automatiquement ces actions dans chaque modèle : l'agent le Dites à l'agent "Partagez cette conception avec l'équipe marketing en tant qu'éditeurs" et il appellera `share-resource` sur le même point de terminaison que celui utilisé par UI. Le résultat apparaît dans la boîte de dialogue de partage lors du prochain rendu. +`unshare-resource` renvoie `ok: true` après une révocation réussie. Pour une autorisation directe d’un utilisateur supprimée, `recipientInvalidation.status` vaut `recorded` si le signal générique d’actualisation du destinataire est enregistré, ou `unconfirmed` si cet enregistrement ne peut pas être confirmé ; aucun ne garantit la réception. `not_applicable` signifie qu’aucune autorisation directe d’utilisateur n’a été supprimée. L’incertitude de notification n’annule pas la révocation. Le signal ne contient ni identifiant, ni titre, ni contenu de ressource. + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/hi-IN/sharing.mdx b/packages/core/docs/content/locales/hi-IN/sharing.mdx index f54135452ab..45a68f937aa 100644 --- a/packages/core/docs/content/locales/hi-IN/sharing.mdx +++ b/packages/core/docs/content/locales/hi-IN/sharing.mdx @@ -147,6 +147,8 @@ import { ShareButton } from "@agent-native/core/client/sharing"; एजेंट को बताएं "इस डिज़ाइन को संपादकों के रूप में मार्केटिंग टीम के साथ साझा करें" और यह UI द्वारा उपयोग किए जाने वाले उसी एंडपॉइंट के विरुद्ध `share-resource` को कॉल करता है। परिणाम अगले रेंडर पर शेयर डायलॉग में दिखाई देता है। +सफल निरस्तीकरण के बाद `unshare-resource`, `ok: true` लौटाता है। हटाई गई प्रत्यक्ष उपयोगकर्ता अनुमति के लिए, प्राप्तकर्ता का सामान्य रीफ़्रेश मार्कर दर्ज होने पर `recipientInvalidation.status` का मान `recorded` होता है; रिकॉर्डिंग की पुष्टि न होने पर `unconfirmed` होता है। दोनों में से कोई भी डिलीवरी की गारंटी नहीं देता। `not_applicable` का अर्थ है कि कोई प्रत्यक्ष उपयोगकर्ता अनुमति नहीं हटाई गई। सूचना की अनिश्चितता निरस्तीकरण को पूर्ववत नहीं करती। मार्कर में संसाधन ID, शीर्षक या सामग्री नहीं होती। + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/ja-JP/sharing.mdx b/packages/core/docs/content/locales/ja-JP/sharing.mdx index ee7c9accb43..c6085bb46c8 100644 --- a/packages/core/docs/content/locales/ja-JP/sharing.mdx +++ b/packages/core/docs/content/locales/ja-JP/sharing.mdx @@ -147,6 +147,8 @@ import { ShareButton } from "@agent-native/core/client/sharing"; エージェントに「このデザインを編集者としてマーケティング チームと共有する」ように指示すると、エージェントは UI が使用するのと同じエンドポイントに対して `share-resource` を呼び出します。結果は、次回のレンダリング時に共有ダイアログに表示されます。 +`unshare-resource` は取り消しが成功すると `ok: true` を返します。直接のユーザー権限が削除された場合、受信者向けの汎用更新マーカーが記録されると `recipientInvalidation.status` は `recorded`、記録を確認できない場合は `unconfirmed` になります。どちらも配信を保証しません。`not_applicable` は直接のユーザー権限が削除されなかったことを示します。通知の不確実性によって取り消しが元に戻ることはありません。マーカーにリソース ID、タイトル、内容は含まれません。 + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/ko-KR/sharing.mdx b/packages/core/docs/content/locales/ko-KR/sharing.mdx index 4ae10ce2f4b..b3405c99e0e 100644 --- a/packages/core/docs/content/locales/ko-KR/sharing.mdx +++ b/packages/core/docs/content/locales/ko-KR/sharing.mdx @@ -147,6 +147,8 @@ import { ShareButton } from "@agent-native/core/client/sharing"; 에이전트에게 "이 디자인을 편집자로서 마케팅 팀과 공유"하라고 말하면 에이전트는 UI가 사용하는 동일한 엔드포인트에 대해 `share-resource`를 호출합니다. 결과는 다음 렌더링 시 공유 대화상자에 표시됩니다. +`unshare-resource`는 권한 취소에 성공하면 `ok: true`를 반환합니다. 직접 사용자 권한이 제거된 경우 수신자의 일반 새로 고침 마커가 기록되면 `recipientInvalidation.status`는 `recorded`, 기록을 확인할 수 없으면 `unconfirmed`입니다. 어느 값도 전달을 보장하지 않습니다. `not_applicable`은 직접 사용자 권한이 제거되지 않았음을 뜻합니다. 알림 상태가 불확실해도 권한 취소는 되돌려지지 않습니다. 마커에는 리소스 ID, 제목 또는 내용이 포함되지 않습니다. + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/pt-BR/sharing.mdx b/packages/core/docs/content/locales/pt-BR/sharing.mdx index 63fb68f3c48..8405d1607c9 100644 --- a/packages/core/docs/content/locales/pt-BR/sharing.mdx +++ b/packages/core/docs/content/locales/pt-BR/sharing.mdx @@ -147,6 +147,8 @@ A estrutura monta automaticamente esses actions em cada modelo — o agente os c Diga ao agente "compartilhe este design com a equipe de marketing como editores" e ele chamará `share-resource` no mesmo endpoint que UI usa. O resultado aparece na caixa de diálogo de compartilhamento na próxima renderização. +`unshare-resource` retorna `ok: true` após uma revogação bem-sucedida. Para uma permissão direta de usuário removida, `recipientInvalidation.status` é `recorded` quando o sinal genérico de atualização do destinatário é registrado, ou `unconfirmed` quando o registro não pode ser confirmado; nenhum garante a entrega. `not_applicable` significa que nenhuma permissão direta de usuário foi removida. A incerteza da notificação não desfaz a revogação. O sinal não contém ID, título ou conteúdo do recurso. + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/zh-CN/sharing.mdx b/packages/core/docs/content/locales/zh-CN/sharing.mdx index 855a670e84f..b6b3c821116 100644 --- a/packages/core/docs/content/locales/zh-CN/sharing.mdx +++ b/packages/core/docs/content/locales/zh-CN/sharing.mdx @@ -147,6 +147,8 @@ import { ShareButton } from "@agent-native/core/client/sharing"; 告诉代理“与作为编辑的营销团队共享此设计”，它会针对 UI 使用的同一端点调用 `share-resource`。结果将显示在下一次渲染的共享对话框中。 +`unshare-resource` 在成功撤销后返回 `ok: true`。对于已移除的直接用户授权，若接收者的通用刷新标记已记录，`recipientInvalidation.status` 为 `recorded`；若无法确认记录，则为 `unconfirmed`。两者均不保证送达。`not_applicable` 表示未移除直接用户授权。通知状态不确定不会撤销已完成的权限撤销。标记不包含资源 ID、标题或内容。 + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/locales/zh-TW/sharing.mdx b/packages/core/docs/content/locales/zh-TW/sharing.mdx index 8bb533c66eb..807ac5dfec6 100644 --- a/packages/core/docs/content/locales/zh-TW/sharing.mdx +++ b/packages/core/docs/content/locales/zh-TW/sharing.mdx @@ -147,6 +147,8 @@ import { ShareButton } from "@agent-native/core/client/sharing"; 告訴代理“與作為編輯的行銷團隊共用此設計”，它會針對 UI 使用的同一端點呼叫 `share-resource`。結果將顯示在下一次渲染的共用對話框中。 +`unshare-resource` 在成功撤銷後傳回 `ok: true`。對於已移除的直接使用者授權，若接收者的通用重新整理標記已記錄，`recipientInvalidation.status` 為 `recorded`；若無法確認記錄，則為 `unconfirmed`。兩者均不保證送達。`not_applicable` 表示未移除直接使用者授權。通知狀態不確定不會復原已完成的權限撤銷。標記不包含資源 ID、標題或內容。 + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. diff --git a/packages/core/docs/content/sharing.mdx b/packages/core/docs/content/sharing.mdx index 47e2efd18db..388c060c7d8 100644 --- a/packages/core/docs/content/sharing.mdx +++ b/packages/core/docs/content/sharing.mdx @@ -165,6 +165,8 @@ The framework auto-mounts these actions in every template — the agent calls th Tell the agent "share this design with the marketing team as editors" and it calls `share-resource` against the same endpoint the UI uses. The result shows up in the share dialog the next render. +`unshare-resource` returns `ok: true` after a successful revocation. For a removed direct-user grant, `recipientInvalidation.status` is `recorded` when the recipient's generic refresh marker is recorded, or `unconfirmed` when recording cannot be confirmed; neither guarantees delivery. `not_applicable` means no direct-user grant was removed. Notification uncertainty does not undo revocation. The marker contains no resource ID, title, or content. + ## Agent-readable share links {#agent-readable-links} Human share links should also work for agents when the resource is a document-like artifact: plans, documents, decks, designs, analytics dashboards, saved analyses, clips, and similar "things you would send to a person." Public links expose a small SSR discovery script that points at structured JSON. Private resources use a short-lived `agent_access` token scoped to exactly one resource. 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/packages/core/src/server/poll-action-marker-replay.spec.ts b/packages/core/src/server/poll-action-marker-replay.spec.ts index e2d65f87d87..3b5af9b4a3c 100644 --- a/packages/core/src/server/poll-action-marker-replay.spec.ts +++ b/packages/core/src/server/poll-action-marker-replay.spec.ts @@ -115,6 +115,38 @@ describe("action marker replay on cold start", () => { expect(db.persisted.map((p) => p.owner)).toContain("recent@x.com"); }); + it("replays recipient revoke markers only to that user regardless of active org", async () => { + const db = makeDb([ + { + session: "recipient@example.test", + ts: NOW - 5_000, + action: "unshare-resource", + }, + ]); + const state = new AppSyncState({ getDb: () => db.exec as never }); + await state.seedVersionFromDb(); + await state.checkExternalDbChanges({ durableEvents: false }); + expect(db.persisted).toEqual([ + expect.objectContaining({ + key: "unshare-resource", + owner: "recipient@example.test", + }), + ]); + expect( + state.getChangesSinceForUser(0, "recipient@example.test", "different-org") + .events, + ).toEqual([ + expect.objectContaining({ + key: "unshare-resource", + owner: "recipient@example.test", + }), + ]); + expect( + state.getChangesSinceForUser(0, "other@example.test", "different-org") + .events, + ).toEqual([]); + }); + it("still replays a marker written just before boot", async () => { // The rewind exists so a separate action process's write is not missed by // the first poll. Bounding the window must not break that. diff --git a/packages/core/src/server/poll-events.spec.ts b/packages/core/src/server/poll-events.spec.ts index 07e390320ab..69fa3043e73 100644 --- a/packages/core/src/server/poll-events.spec.ts +++ b/packages/core/src/server/poll-events.spec.ts @@ -73,6 +73,56 @@ describe("poll event SSE handler", () => { event.close?.(); }); + it("routes recipient revoke invalidations across orgs without exposing them to other users", async () => { + const { createPollEventsHandler } = await import("./poll-events.js"); + const { recordChange, getVersion, getChangesSinceForUser } = + await import("./poll.js"); + const handler = createPollEventsHandler() as any; + const recipient = { pushed: [] as unknown[], close: undefined as any }; + const outsider = { pushed: [] as unknown[], close: undefined as any }; + mockSession.value = { + email: "recipient@example.test", + orgId: "recipient-org", + }; + await handler(recipient); + mockSession.value = { email: "other@example.test", orgId: "recipient-org" }; + await handler(outsider); + const before = getVersion(); + try { + recordChange({ + source: "action", + type: "change", + key: "unshare-resource", + owner: "recipient@example.test", + }); + const messages = recipient.pushed.filter( + (value) => typeof value === "string", + ); + expect(messages).toHaveLength(1); + const change = JSON.parse(messages[0] as string); + expect(change).toMatchObject({ + key: "unshare-resource", + owner: "recipient@example.test", + }); + expect(change).not.toHaveProperty("resourceId"); + expect(change).not.toHaveProperty("orgId"); + expect( + outsider.pushed.filter((value) => typeof value === "string"), + ).toHaveLength(0); + expect( + getChangesSinceForUser(before, "recipient@example.test", "another-org") + .events, + ).toHaveLength(1); + expect( + getChangesSinceForUser(before, "other@example.test", "recipient-org") + .events, + ).toHaveLength(0); + } finally { + recipient.close?.(); + outsider.close?.(); + } + }); + it("sends named heartbeats while the stream is idle", async () => { vi.useFakeTimers(); try { diff --git a/packages/core/src/sharing/access.spec.ts b/packages/core/src/sharing/access.spec.ts index 23e549dfde6..235e0787c0a 100644 --- a/packages/core/src/sharing/access.spec.ts +++ b/packages/core/src/sharing/access.spec.ts @@ -4,6 +4,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { createTestPglite } from "../a2a/test-pglite.js"; import { table, text, ownableColumns } from "../db/schema.js"; +import { notifyActionChange } from "../server/action-change.js"; import { runWithRequestContext } from "../server/request-context.js"; import { accessFilter, @@ -27,6 +28,10 @@ import { type ShareRole, } from "./schema.js"; +vi.mock("../server/action-change.js", () => ({ + notifyActionChange: vi.fn(async () => {}), +})); + const resourceType = "qa-doc"; const ownerEmail = "owner+qa@example.com"; const viewerEmail = "viewer+qa@example.com"; @@ -87,6 +92,7 @@ async function listVisible( } beforeEach(async () => { + vi.mocked(notifyActionChange).mockReset().mockResolvedValue(undefined); pglite = await createTestPglite(); await pglite.exec(` CREATE TABLE qa_docs ( @@ -817,7 +823,10 @@ describe("shareable resource access helpers", () => { principalType: "org", principalId: otherOrgId, }), - ).resolves.toEqual({ ok: true }); + ).resolves.toEqual({ + ok: true, + recipientInvalidation: { status: "not_applicable" }, + }); }); const shares = await db @@ -917,7 +926,10 @@ describe("shareable resource access helpers", () => { principalType: "user", principalId: "VIEWER+QA@EXAMPLE.COM", }), - ).resolves.toEqual({ ok: true }); + ).resolves.toEqual({ + ok: true, + recipientInvalidation: { status: "recorded" }, + }); }); shares = await db @@ -925,6 +937,89 @@ describe("shareable resource access helpers", () => { .from(docShares) .where(eq(docShares.resourceId, "doc-user-case-actions")); expect(shares).toHaveLength(0); + expect(notifyActionChange).toHaveBeenCalledExactlyOnceWith({ + actionName: "unshare-resource", + owner: viewerEmail, + }); + }); + + it("does not notify an absent grant or an unauthorized revoke", async () => { + await insertDoc({ id: "doc-revoke-absent" }); + await db.insert(docShares).values({ + id: "revoke-denied-share", + resourceId: "doc-revoke-absent", + principalType: "user", + principalId: viewerEmail, + role: "viewer", + createdBy: ownerEmail, + createdAt: new Date().toISOString(), + }); + const args = { + resourceType, + resourceId: "doc-revoke-absent", + principalType: "user" as const, + principalId: viewerEmail, + }; + await expect( + runWithRequestContext( + { userEmail: outsiderEmail, orgId: otherOrgId }, + () => unshareResource.run(args), + ), + ).rejects.toThrow(); + expect(await db.select().from(docShares)).toHaveLength(1); + expect(notifyActionChange).not.toHaveBeenCalled(); + await db.delete(docShares); + await expect( + runWithRequestContext({ userEmail: ownerEmail, orgId }, () => + unshareResource.run(args), + ), + ).resolves.toEqual({ + ok: true, + recipientInvalidation: { status: "not_applicable" }, + }); + expect(notifyActionChange).not.toHaveBeenCalled(); + }); + + it("acknowledges the deleted grant when recipient invalidation is unconfirmed", async () => { + await insertDoc({ id: "doc-revoke-marker-failure" }); + await db.insert(docShares).values({ + id: "revoke-marker-share", + resourceId: "doc-revoke-marker-failure", + principalType: "user", + principalId: viewerEmail, + role: "viewer", + createdBy: ownerEmail, + createdAt: new Date().toISOString(), + }); + const failure = new Error("Marker write unavailable"); + vi.mocked(notifyActionChange).mockRejectedValueOnce(failure); + const diagnostic = vi.spyOn(console, "error").mockImplementation(() => {}); + try { + await expect( + runWithRequestContext({ userEmail: ownerEmail, orgId }, () => + unshareResource.run({ + resourceType, + resourceId: "doc-revoke-marker-failure", + principalType: "user", + principalId: viewerEmail, + }), + ), + ).resolves.toEqual({ + ok: true, + recipientInvalidation: { status: "unconfirmed" }, + }); + expect(await db.select().from(docShares)).toHaveLength(0); + expect(diagnostic).toHaveBeenCalledWith( + "[unshare-resource] recipient invalidation", + { + event: "recipient_invalidation_unconfirmed", + status: "unconfirmed", + error: failure, + }, + ); + } finally { + diagnostic.mockRestore(); + } }); it("rejects non-email user share principals", async () => { diff --git a/packages/core/src/sharing/actions/unshare-resource.ts b/packages/core/src/sharing/actions/unshare-resource.ts index b99b7b8b2ed..35ce8a47b0c 100644 --- a/packages/core/src/sharing/actions/unshare-resource.ts +++ b/packages/core/src/sharing/actions/unshare-resource.ts @@ -2,6 +2,7 @@ import { and, eq, sql, type SQL } from "drizzle-orm"; import { z } from "zod"; import { defineAction } from "../../action.js"; +import { notifyActionChange } from "../../server/action-change.js"; import { invalidateCollabAccessCache } from "../../server/poll.js"; import { assertAccess } from "../access.js"; import { requireShareableResource } from "../registry.js"; @@ -10,6 +11,14 @@ import { notifyExtensionShareChanged, } from "./extension-change.js"; +export interface UnshareResourceResult { + ok: true; + /** Marker persistence is acknowledged separately from the committed revoke. */ + recipientInvalidation: { + status: "recorded" | "unconfirmed" | "not_applicable"; + }; +} + function normalizePrincipalId( principalType: "user" | "group" | "org", principalId: string, @@ -40,7 +49,7 @@ export default defineAction({ principalType: z.enum(["user", "group", "org"]), principalId: z.string(), }), - run: async (args) => { + run: async (args): Promise => { const reg = requireShareableResource(args.resourceType); await assertAccess(args.resourceType, args.resourceId, "admin"); const beforeExtensionTargets = await getExtensionShareChangeTargets( @@ -52,7 +61,7 @@ export default defineAction({ args.principalType, args.principalId, ); - await db + const removed = await db .delete(reg.sharesTable) .where( and( @@ -60,13 +69,34 @@ export default defineAction({ eq(reg.sharesTable.principalType, args.principalType), principalIdMatches(reg.sharesTable, args.principalType, principalId), ), - ); + ) + .returning({ principalId: reg.sharesTable.principalId }); invalidateCollabAccessCache(args.resourceType, args.resourceId); await notifyExtensionShareChanged( args.resourceType, args.resourceId, beforeExtensionTargets, ); - return { ok: true }; + let status: UnshareResourceResult["recipientInvalidation"]["status"] = + "not_applicable"; + if (args.principalType === "user" && removed.length > 0) { + try { + await notifyActionChange({ + actionName: "unshare-resource", + owner: principalId, + }); + status = "recorded"; + } catch (error) { + // The grant is already deleted; a marker failure must not roll back + // the caller's revoked-share UI or imply the grant still exists. + status = "unconfirmed"; + console.error("[unshare-resource] recipient invalidation", { + event: "recipient_invalidation_unconfirmed", + status, + error, + }); + } + } + return { ok: true, recipientInvalidation: { status } }; }, }); 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..156c1c720a8 100644 --- a/templates/content/actions/_document-edit-mutation.db.test.ts +++ b/templates/content/actions/_document-edit-mutation.db.test.ts @@ -2,7 +2,8 @@ import { rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { eq } from "drizzle-orm"; +import { runWithRequestContext } from "@agent-native/core/server"; +import { and, eq } from "drizzle-orm"; import { afterAll, beforeAll, beforeEach, describe, expect, it } from "vitest"; const TEST_DB_PATH = join( @@ -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,7 +36,10 @@ beforeEach(async () => { const db = getDb(); await db.delete(schema.documentEditReceipts); await db.delete(schema.documentVersions); - await db.delete(schema.documents); + await db.delete(schema.documentShares); + await db + .delete(schema.documents) + .where(eq(schema.documents.ownerEmail, OWNER)); await db.insert(schema.documents).values({ id: DOCUMENT_ID, ownerEmail: OWNER, @@ -47,6 +56,41 @@ 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( + and( + eq(schema.documents.id, DOCUMENT_ID), + eq(schema.documents.ownerEmail, OWNER), + ), + ); + 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) + .where(eq(schema.documents.ownerEmail, OWNER)) + )[0].content, + ).toBe("alpha beta"); + }); + it("commits one revision/version/receipt and replays a double delivery", async () => { const input = { documentId: DOCUMENT_ID, @@ -71,7 +115,12 @@ describe("revisioned document edit mutation", () => { const [document] = await db .select() .from(schema.documents) - .where(eq(schema.documents.id, DOCUMENT_ID)); + .where( + and( + eq(schema.documents.id, DOCUMENT_ID), + eq(schema.documents.ownerEmail, OWNER), + ), + ); expect(document).toMatchObject({ content: "omega beta", bodyRevision: 1 }); expect(await db.select().from(schema.documentVersions)).toHaveLength(2); expect(await db.select().from(schema.documentEditReceipts)).toHaveLength(1); @@ -139,6 +188,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"), @@ -173,7 +230,7 @@ describe("revisioned document edit mutation", () => { BEGIN UPDATE documents SET content = 'legacy writer won' - WHERE id = '${DOCUMENT_ID}'; + WHERE owner_email = 'document-editor@example.com' AND id = '${DOCUMENT_ID}'; RETURN NEW; END; $legacy$ @@ -316,7 +373,12 @@ describe("revisioned document edit mutation", () => { const [document] = await getDb() .select() .from(schema.documents) - .where(eq(schema.documents.id, DOCUMENT_ID)); + .where( + and( + eq(schema.documents.id, DOCUMENT_ID), + eq(schema.documents.ownerEmail, OWNER), + ), + ); expect(document.content).toBe("beta gamma"); }); @@ -324,7 +386,12 @@ describe("revisioned document edit mutation", () => { await getDb() .update(schema.documents) .set({ content: "legacy writer changed beta" }) - .where(eq(schema.documents.id, DOCUMENT_ID)); + .where( + and( + eq(schema.documents.id, DOCUMENT_ID), + eq(schema.documents.ownerEmail, OWNER), + ), + ); await expect( mutateDocumentBody({ documentId: DOCUMENT_ID, 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..48e411aaf1f 100644 --- a/templates/content/actions/blocks-seeding.db.test.ts +++ b/templates/content/actions/blocks-seeding.db.test.ts @@ -127,6 +127,194 @@ 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/content-database-lifecycle.db.test.ts b/templates/content/actions/content-database-lifecycle.db.test.ts index 56dfb33608a..edc5cc6e68e 100644 --- a/templates/content/actions/content-database-lifecycle.db.test.ts +++ b/templates/content/actions/content-database-lifecycle.db.test.ts @@ -993,7 +993,7 @@ describe("document trash lifecycle", () => { runWithRequestContext({ userEmail: OWNER }, () => getDocumentAction.run({ id: rootId }), ), - ).rejects.toMatchObject({ statusCode: 404 }); + ).rejects.toMatchObject({ statusCode: 409, errorCode: "DOCUMENT_TRASHED" }); const trash = await runWithRequestContext({ userEmail: OWNER }, () => listTrashedDocumentsAction.run({}), @@ -1804,12 +1804,12 @@ describe("content database soft-delete actions and reads", () => { runWithRequestContext({ userEmail: OWNER }, () => getDocumentAction.run({ id: databaseDocumentId }), ), - ).rejects.toThrow(`Document "${databaseDocumentId}" not found`); + ).rejects.toMatchObject({ statusCode: 409, errorCode: "DOCUMENT_TRASHED" }); await expect( runWithRequestContext({ userEmail: OWNER }, () => getDocumentAction.run({ id: rowDocumentId }), ), - ).rejects.toThrow(`Document "${rowDocumentId}" not found`); + ).rejects.toMatchObject({ statusCode: 409, errorCode: "DOCUMENT_TRASHED" }); await expect( runWithRequestContext({ userEmail: OWNER }, () => pullDocumentAction.run({ id: rowDocumentId, format: "markdown" }), 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..e20185fa489 100644 --- a/templates/content/actions/get-document.ts +++ b/templates/content/actions/get-document.ts @@ -20,6 +20,7 @@ import { documentContentHash, documentRevisionToken, } from "./_document-edit-mutation.js"; +import { documentTrashedError } from "./_document-lifecycle.js"; import { serializeDocumentSource } from "./_document-source.js"; import { getDatabaseById, @@ -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", + )}