feat(yjs): improve handling of external file conflicts

dev/docs/yjs.md

@@ -314,10 +314,22 @@ convergence. A peer that joins while `isStale` is already up offers itself the s
 `nativeEtag` - the observer only fires on change, so without that the room would stay stuck if the elected peer
 navigated away mid-recovery.
 
+Peers receive the rewrite as ordinary CRDT updates, and a peer holding unsaved work would lose it without a word. So a
+peer that is dirty when a _remote_ `isStale` goes up does not follow: it marks itself conflicted, stops its provider
+and keeps its own Y.Doc. The editor still shows that work and the user sees a conflict message. A reload rejoins.
+
+Note that an external write drops _every_ peer in a room that holds any unsaved state, since the dirty state is shared
+across all peers. That is deliberate: a peer cannot tell whose edits the recovery would discard, and losing them silently
+is worse than a conflict message.
+
+A peer that is _clean_ follows the recovery, and the commit stamps `savedStateVector` and `lastSavedAt` alongside the
+fresh `etag`.
+
 Reset lands before hydrate, so a throw in between leaves every peer looking at an empty document. That path keeps
 `isStale` up for the next joiner to retry, and locks the session so the editor freezes. Note that the lock does _not_
-block saving - `isDirty` deliberately ignores it so a lock cannot silently discard unsaved work (see Known limits). What
-actually stops an empty document reaching disk is `serializeDoc` returning `null` when the adapter reports no content.
+block saving - `isDirty` deliberately ignores it so a lock cannot silently discard unsaved work (see Known limits).
+What actually stops an empty document reaching disk is `serializeDoc` returning `null` when the adapter reports no
+content.
 
 ### `_oc_meta`
 
@@ -326,8 +338,8 @@ A `Y.Map` alongside the editor content, used for coordination the editor never s
 | Key                | Written by          | Meaning                                       |
 | ------------------ | ------------------- | --------------------------------------------- |
 | `etag`             | whoever saved last  | the etag the room believes is on disk         |
-| `lastSavedAt`      | whoever saved last  | fan-out trigger for a peer save               |
-| `savedStateVector` | whoever saved last  | what that peer's doc held when it wrote       |
+| `lastSavedAt`      | last save, recovery | fan-out trigger for a peer save               |
+| `savedStateVector` | last save, recovery | what that peer's doc held when it wrote       |
 | `isStale`          | any writer          | the file changed outside this room; rehydrate |
 | `nativeEtag`       | writer that noticed | the etag recovery should settle on            |
 | `recoveryClientId` | writer that noticed | which peer is elected to re-seed the room     |
@@ -462,10 +474,10 @@ public-link tokens, so any context without a user access token - a public link,
 `yjsServerUrl`. Editing works, it just does not sync. A signed-in visitor opening someone else's public link is treated
 the same way, since their token carries no grant on the shared file.
 
-**Connection state is invisible.** The session exposes a `status` ref (`connecting` / `connected` / `disconnected` /
-`local`). `AppWrapper` reads it to decide whether a save conflict can be reconciled, but never shows it. A websocket
-that drops _after_ a successful sync produces no indicator: the user keeps typing into a Y.Doc that no longer reaches
-anyone. Only a connect that never succeeds in the first place is caught, by the timeout above.
+**A conflicted peer stays out until reload.** Leaving the room protects unsaved work, but there is no way back inside
+the session: the provider is stopped for good, so the user keeps editing a document nobody else sees until they save
+elsewhere and reload. Re-joining would mean merging a diverged doc into a room that has moved on, which the CRDT cannot
+do meaningfully here.
 
 **Conflict reconciliation depends on a timely etag stamp.** While connected, a 409/412 is only retried when the room can
 account for the write: the fresh etag must match `_oc_meta.etag`, with a short grace period (`ROOM_ETAG_GRACE_MS`) for a

packages/web-pkg/src/components/AppTemplates/AppWrapper.vue

@@ -229,16 +229,25 @@ const {
   applicationId
 })
 
+const isDirty = computed(() => {
+  if (unref(isReadOnly)) {
+    return false
+  }
+  return unref(currentContent) !== unref(serverContent)
+})
+
 const {
   session: yjsSession,
   isSessionReady,
   effectiveReadOnly,
+  isConflicted,
   reconcileConflict
 } = useAppWrapperYjs({
   yjs,
   applicationId,
   resource,
   isReadOnly,
+  isDirty,
   loading,
   loadingError,
   contentResourceId,
@@ -249,7 +258,8 @@ const {
   fileContentOptions,
   getFileContents,
   putFileContents,
-  applySavedResource
+  applySavedResource,
+  onConflict: showExternalUpdateConflict
 })
 
 // Keep the loading screen up until the Yjs session is synced and hydrated,
@@ -261,18 +271,6 @@ const isLoading = computed(() => {
   return unref(loading) || !unref(isSessionReady)
 })
 
-const isDirty = computed(() => {
-  // Peer edits flow into `currentContent` of a read-only client too, but it
-  // has nothing to save, so it must never be prompted. Deliberately the
-  // WebDAV permission and not `effectiveReadOnly`: a session locking mid-edit
-  // may hold real unsaved work, which must keep the save action and the
-  // leave guards armed.
-  if (unref(isReadOnly)) {
-    return false
-  }
-  return unref(currentContent) !== unref(serverContent)
-})
-
 watch(isDirty, (dirty) => {
   // Prevent reload if there are changes
   if (dirty) {
@@ -516,6 +514,17 @@ const autosavePopup = () => {
   showMessage({ title: $gettext('File autosaved') })
 }
 
+function showExternalUpdateConflict(response: Response = null) {
+  errorPopup(
+    new HttpError(
+      $gettext(
+        'This file was updated outside this window. Please copy your changes, save the file under a new name (»Save As...«) or reload the page to discard your changes.'
+      ),
+      response
+    )
+  )
+}
+
 /**
  * Single landing point for a successful write: updates the etag for the next
  * `If-Match`, the local `resource` ref and the store. `saved` is the PUT's
@@ -556,14 +565,7 @@ const saveFileTask = useTask(function* () {
       if (yield* reconcileConflict(newContent)) {
         return
       }
-      errorPopup(
-        new HttpError(
-          $gettext(
-            'This file was updated outside this window. Please copy your changes or save the file under a new name (»Save As...«).'
-          ),
-          e.response
-        )
-      )
+      showExternalUpdateConflict(e.response)
       return
     }
     switch (e.statusCode) {
@@ -630,10 +632,11 @@ onMounted(() => {
   if (editorOptions.autosaveEnabled && !disableAutoSave) {
     autosaveIntervalId = setInterval(
       async () => {
-        if (isDirty.value) {
-          await save()
-          autosavePopup()
+        if (!unref(isDirty) || unref(isConflicted)) {
+          return
         }
+        await save()
+        autosavePopup()
       },
       (editorOptions.autosaveInterval || 120) * 1000
     )
@@ -700,8 +703,18 @@ const downloadFileActionInterceptor = async (
   originalAction: Action<FileActionOptions>['handler']
 ) => {
   if (unref(isDirty)) {
-    await save()
-    autosavePopup()
+    if (unref(isConflicted)) {
+      showMessage({
+        title: $gettext('Unsaved changes are not included'),
+        desc: $gettext(
+          'This file was updated outside this window, so the download does not contain your unsaved changes.'
+        ),
+        status: 'warning'
+      })
+    } else {
+      await save()
+      autosavePopup()
+    }
   }
   originalAction(args)
 }

packages/web-pkg/src/components/AppTemplates/useAppWrapperYjs.ts

@@ -3,7 +3,7 @@ import type { Ref } from 'vue'
 import { useGettext } from 'vue3-gettext'
 import { call } from '@opencloud-eu/web-client'
 import type { Resource } from '@opencloud-eu/web-client'
-import { useMessages, useYjsSession } from '../../composables'
+import { useMessages, useYjsSession, YjsStatus } from '../../composables'
 import type { AppFileHandlingResult, FileContentOptions, FileContext } from '../../composables'
 import type { YjsOptions } from './types'
 
@@ -13,6 +13,8 @@ export interface AppWrapperYjsOptions {
   applicationId: string
   resource: Ref<Resource>
   isReadOnly: Ref<boolean>
+  /** Whether the wrapper holds edits that are not on disk yet. */
+  isDirty: Ref<boolean>
   loading: Ref<boolean>
   loadingError: Ref<Error>
   contentResourceId: Ref<string | undefined>
@@ -25,6 +27,8 @@ export interface AppWrapperYjsOptions {
   putFileContents: AppFileHandlingResult['putFileContents']
   /** AppWrapper's landing point for a successful write. */
   applySavedResource: (etag: string, saved?: Resource | null) => void
+  /** The file changed outside this window and the session gave up the room. */
+  onConflict: () => void
 }
 
 /**
@@ -38,6 +42,7 @@ export function useAppWrapperYjs(options: AppWrapperYjsOptions) {
     applicationId,
     resource,
     isReadOnly,
+    isDirty,
     loading,
     loadingError,
     contentResourceId,
@@ -48,7 +53,8 @@ export function useAppWrapperYjs(options: AppWrapperYjsOptions) {
     fileContentOptions,
     getFileContents,
     putFileContents,
-    applySavedResource
+    applySavedResource,
+    onConflict
   } = options
 
   const { $gettext } = useGettext()
@@ -85,7 +91,9 @@ export function useAppWrapperYjs(options: AppWrapperYjsOptions) {
           // saves itself would otherwise read the next reconnect as one and
           // wipe the room to "recover" it.
           resource.value = { ...unref(resource), etag: value }
-        }
+        },
+        hasUnsavedChanges: () => unref(isDirty),
+        onConflict
       })
     : null
 
@@ -113,7 +121,10 @@ export function useAppWrapperYjs(options: AppWrapperYjsOptions) {
   // TNext must be `any`: the delegated `call()` generators expect different
   // resume types, which would otherwise intersect to `never`.
   function* reconcileConflict(newContent: unknown): Generator<unknown, boolean, any> {
-    if (!session || unref(session.status) !== 'connected') return false
+    if (!session) return false
+    // Not connected: nothing in the room can account for the write, so this is
+    // an external change like any other.
+    if (unref(session.status) !== YjsStatus.Connected) return false
     try {
       const fresh = yield* call(getFileContents(currentFileContext, { ...fileContentOptions }))
       const freshEtag = fresh.headers['OC-ETag']
@@ -129,7 +140,10 @@ export function useAppWrapperYjs(options: AppWrapperYjsOptions) {
       // republished over: a write from outside the room never reached this
       // document, and retrying would destroy it unseen.
       const writtenByRoom = yield* call(session.wasWrittenByRoom(freshEtag))
-      if (!writtenByRoom) return false
+      if (!writtenByRoom) {
+        session.markConflicted(false)
+        return false
+      }
 
       // Retry with the merged state, not `newContent`: that snapshot predates
       // the peer save that caused this conflict, so writing it again would
@@ -151,11 +165,15 @@ export function useAppWrapperYjs(options: AppWrapperYjsOptions) {
       return true
     } catch (retryErr) {
       // Fall through to the conflict popup so the user can still recover by
-      // copying out.
+      // copying out. The room stays: the cause is just as likely a failed
+      // request as an external write, and a retry may well go through.
       console.error('[yjs] conflict reconciliation failed:', retryErr)
       return false
     }
   }
 
-  return { session, isSessionReady, effectiveReadOnly, reconcileConflict }
+  /** True once the session gave up the room over an external write. */
+  const isConflicted = computed(() => Boolean(unref(session?.isConflicted)))
+
+  return { session, isSessionReady, effectiveReadOnly, isConflicted, reconcileConflict }
 }