diff --git a/frontend/deterministic-tests/src/tests/17-create-merge-preserves-renamed-update.test.ts b/frontend/deterministic-tests/src/tests/17-create-merge-preserves-renamed-update.test.ts index f2b6ba62..a9bc37d4 100644 --- a/frontend/deterministic-tests/src/tests/17-create-merge-preserves-renamed-update.test.ts +++ b/frontend/deterministic-tests/src/tests/17-create-merge-preserves-renamed-update.test.ts @@ -15,6 +15,13 @@ export const createMergePreservesRenamedUpdateTest: TestDefinition = { { type: "enable-sync", client: 1 }, { type: "barrier" }, + { + type: "assert-consistent", + verify: (state: AssertableState): void => { + state.assertContains("doc.md", "alpha", "beta"); + } + }, + { type: "disable-sync", client: 1 }, { diff --git a/frontend/deterministic-tests/src/tests/7-concurrent-rename-and-create-at-target.test.ts b/frontend/deterministic-tests/src/tests/7-concurrent-rename-and-create-at-target.test.ts index fea90dd9..e142a020 100644 --- a/frontend/deterministic-tests/src/tests/7-concurrent-rename-and-create-at-target.test.ts +++ b/frontend/deterministic-tests/src/tests/7-concurrent-rename-and-create-at-target.test.ts @@ -13,6 +13,8 @@ export const concurrentRenameAndCreateAtTargetTest: TestDefinition = { path: "X.md", content: "original file X" }, + { type: "enable-sync", client: 0 }, + { type: "enable-sync", client: 1 }, { type: "barrier" }, { type: "disable-sync", client: 0 }, diff --git a/frontend/deterministic-tests/src/tests/online-create-update-while-other-creates-same-path.test.ts b/frontend/deterministic-tests/src/tests/online-create-update-while-other-creates-same-path.test.ts index 68a64e9f..e0ddc21a 100644 --- a/frontend/deterministic-tests/src/tests/online-create-update-while-other-creates-same-path.test.ts +++ b/frontend/deterministic-tests/src/tests/online-create-update-while-other-creates-same-path.test.ts @@ -39,8 +39,9 @@ export const onlineCreateUpdateWhileOtherCreatesSamePathTest: TestDefinition = { verify: (state: AssertableState): void => { state .assertFileCount(2) - .assertContains("data.bin", "content-v2") - .assertContains("data (1).bin", "other-content"); + .assertNoFileContains("content-v1") + .assertAnyFileContains("content-v2") + .assertAnyFileContains("other-content"); } } ] diff --git a/frontend/deterministic-tests/src/tests/reset-clears-recently-deleted-resurrection.test.ts b/frontend/deterministic-tests/src/tests/reset-clears-recently-deleted-resurrection.test.ts index 5b14256a..e0a1565c 100644 --- a/frontend/deterministic-tests/src/tests/reset-clears-recently-deleted-resurrection.test.ts +++ b/frontend/deterministic-tests/src/tests/reset-clears-recently-deleted-resurrection.test.ts @@ -16,13 +16,9 @@ export const resetClearsRecentlyDeletedResurrectionTest: TestDefinition = { }, { type: "enable-sync", client: 0 }, { type: "enable-sync", client: 1 }, - { type: "sync" }, { type: "barrier" }, { type: "delete", client: 0, path: "ghost.md" }, - { type: "sync", client: 0 }, - - { type: "sync" }, { type: "barrier" }, { @@ -34,7 +30,7 @@ export const resetClearsRecentlyDeletedResurrectionTest: TestDefinition = { { type: "disable-sync", client: 1 }, { type: "enable-sync", client: 1 }, - { type: "sync" }, + { type: "barrier" }, { diff --git a/frontend/deterministic-tests/src/tests/server-pause-update-and-create.test.ts b/frontend/deterministic-tests/src/tests/server-pause-update-and-create.test.ts index e10e37d9..2389ccf5 100644 --- a/frontend/deterministic-tests/src/tests/server-pause-update-and-create.test.ts +++ b/frontend/deterministic-tests/src/tests/server-pause-update-and-create.test.ts @@ -14,7 +14,6 @@ export const serverPauseUpdateAndCreateTest: TestDefinition = { path: "shared.md", content: "initial content" }, - { type: "sync" }, { type: "barrier" }, { type: "assert-consistent", @@ -40,7 +39,6 @@ export const serverPauseUpdateAndCreateTest: TestDefinition = { { type: "resume-server" }, - { type: "sync" }, { type: "barrier" }, { diff --git a/frontend/deterministic-tests/src/utils/assertable-state.ts b/frontend/deterministic-tests/src/utils/assertable-state.ts index 196333c0..7c6f192c 100644 --- a/frontend/deterministic-tests/src/utils/assertable-state.ts +++ b/frontend/deterministic-tests/src/utils/assertable-state.ts @@ -86,6 +86,26 @@ export class AssertableState { return this; } + public assertNoFileContains(...substrings: string[]): this { + const offenders: { path: string; substring: string }[] = []; + for (const [path, content] of this.files) { + for (const s of substrings) { + if (content.includes(s)) { + offenders.push({ path, substring: s }); + } + } + } + if (offenders.length > 0) { + const dump = Array.from(this.files.entries()) + .map(([k, v]) => ` ${k}: "${v}"`) + .join("\n"); + throw new Error( + `Expected no file to contain ${substrings.map((s) => `"${s}"`).join(", ")}, but found ${offenders.map((o) => `"${o.substring}" in "${o.path}"`).join(", ")}.\nFiles:\n${dump}` + ); + } + return this; + } + public assertSubstringCount( path: string, substring: string, diff --git a/frontend/sync-client/src/file-operations/file-operations.ts b/frontend/sync-client/src/file-operations/file-operations.ts index 3d11c278..46baf94e 100644 --- a/frontend/sync-client/src/file-operations/file-operations.ts +++ b/frontend/sync-client/src/file-operations/file-operations.ts @@ -29,7 +29,7 @@ export class FileOperations { this.fs = new SafeFileSystemOperations(fs, logger); } - private static getParentDirAndFile( + private static getParentDirAndFileName( path: RelativePath ): [RelativePath, RelativePath] { const pathParts = path.split("/"); @@ -47,7 +47,7 @@ export class FileOperations { * statistically impossible, so no disk probe / lock dance is needed. */ private static buildConflictPath(path: RelativePath): RelativePath { - const [directory, fileName] = FileOperations.getParentDirAndFile(path); + const [directory, fileName] = FileOperations.getParentDirAndFileName(path); const conflictName = buildConflictFileName(fileName); return directory ? `${directory}/${conflictName}` : conflictName; } @@ -202,6 +202,8 @@ export class FileOperations { return this.fs.exists(path); } + + // Returns the actual path the file got moved to. public async move( oldPath: RelativePath, @@ -234,7 +236,12 @@ export class FileOperations { `Displacing existing file at ${path} to '${conflictPath}' to make room` ); - this.expectedFsEvents.expectRename(path, conflictPath); + // Intentionally NOT calling `expectRename` here: the displaced + // file may be a tracked document (its `queue.documents` entry + // still points at `path`), and we need the watcher's + // `syncLocallyUpdatedFile` to flow into `queue.enqueue`'s + // path-update branch so the doc's map key follows its file + // to `conflictPath` and gets resynced await this.fs.rename(path, conflictPath); return path; } @@ -252,7 +259,7 @@ export class FileOperations { let directory = path; // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition while (true) { - [directory] = FileOperations.getParentDirAndFile(directory); + [directory] = FileOperations.getParentDirAndFileName(directory); if (directory.length === 0) { break; } diff --git a/frontend/sync-client/src/sync-client.ts b/frontend/sync-client/src/sync-client.ts index 00dbd245..99a0221f 100644 --- a/frontend/sync-client/src/sync-client.ts +++ b/frontend/sync-client/src/sync-client.ts @@ -435,7 +435,18 @@ export class SyncClient { } public async waitUntilFinished(): Promise { - this.checkIfDestroyed("waitUntilIdle"); + this.checkIfDestroyed("waitUntilFinished"); + await this.waitUntilFinishedInternal(); + } + + /** + * The actual drain — separated from `waitUntilFinished` so internal + * shutdown paths (`pause` / `destroy`) can wait for in-flight work + * without tripping the public `checkIfDestroyed` guard, which exists + * only to keep external callers from continuing to use a disposed + * client. + */ + private async waitUntilFinishedInternal(): Promise { await this.syncer.waitUntilFinished(); await this.webSocketManager.waitUntilFinished(); await this.syncEventQueue.save(); @@ -502,7 +513,7 @@ export class SyncClient { // the rest of the client is winding down. this.syncService.stop(); await this.webSocketManager.stop(); - await this.waitUntilFinished(); + await this.waitUntilFinishedInternal(); // Clear the offline-scan gate so a subsequent `startSyncing()` // re-runs the scan; otherwise any local changes made while sync was // paused (offline edits, deletes, renames) wouldn't be detected, and diff --git a/frontend/sync-client/src/sync-operations/offline-change-detector.ts b/frontend/sync-client/src/sync-operations/offline-change-detector.ts index 368e07ed..b3cb4dd1 100644 --- a/frontend/sync-client/src/sync-operations/offline-change-detector.ts +++ b/frontend/sync-client/src/sync-operations/offline-change-detector.ts @@ -73,17 +73,25 @@ export async function scheduleOfflineChanges( for (const path of locallyPossibleCreatedFiles) { if (renamedPaths.has(path)) continue; - logger.debug( + + logger.info( `File ${path} was created while offline, scheduling sync to create it` ); + enqueueCreate(path); } for (const item of locallyPossiblyDeletedFiles) { + logger.info( + `File ${item.path} was deleted while offline, scheduling sync to delete it` + ); enqueueDelete(item.path); } for (const path of syncedLocalFiles) { + logger.info( + `File ${path} may have been updated while offline, scheduling sync to update it` + ); enqueueUpdate({ relativePath: path }); } } diff --git a/frontend/sync-client/src/sync-operations/sync-event-queue.ts b/frontend/sync-client/src/sync-operations/sync-event-queue.ts index ff8d9b65..d362b533 100644 --- a/frontend/sync-client/src/sync-operations/sync-event-queue.ts +++ b/frontend/sync-client/src/sync-operations/sync-event-queue.ts @@ -38,6 +38,14 @@ export class SyncEventQueue { // It maps pending changes onto the local filesystem. private readonly events: SyncEvent[] = []; + // Tombstones: documents we deleted along with the vaultUpdateId at + // which the delete committed. After we delete, the server may still + // send us older broadcasts for that document (e.g. a backlog update + // committed before the delete from another client). Without these + // entries, the syncer would resurrect the doc by treating an old + // update as a brand-new create. + private readonly deletedDocuments = new Map(); + // file creations for paths matching any of these patterns are ignored // because the user explicitly told us to ignore them. private userIgnorePatterns: RegExp[]; @@ -121,15 +129,29 @@ export class SyncEventQueue { return; } - if (this.ignoreConflictPaths && CONFLICT_PATH_REGEX.test(path)) { - this.logger.info( - `Ignoring ${input.type} for ${path} as it is a conflict path` - ); + if (input.type === SyncEventType.RemoteChange) { + this.events.push(input); return; } - if (input.type === SyncEventType.RemoteChange) { - this.events.push(input); + // Drop bare LocalCreate events for conflict paths. Those are + // produced by the watcher when the syncer's own write to a + // displacement path slips past the `ExpectedFsEvents` filter + // (e.g. a sync race where the watcher fires before + // `expectCreate` was registered). Re-uploading them as new docs + // would invent duplicates on the server. The legitimate way a + // conflict-path doc enters the queue is via the displacement + // rename's `LocalUpdate` (with `oldPath`) — that branch is + // allowed through below so the tracked document's path follows + // its file. + if ( + this.ignoreConflictPaths && + CONFLICT_PATH_REGEX.test(path) && + input.type === SyncEventType.LocalCreate + ) { + this.logger.info( + `Ignoring local-create for ${path} as it is a conflict path` + ); return; } @@ -215,6 +237,31 @@ export class SyncEventQueue { return this.events.shift(); } + + /** + * Return the next event without removing it. Drain uses this so the + * event stays visible in the queue while it is being processed — + * critical for `findLatestCreateForPath` to update an in-flight + * `LocalCreate`'s path when a rename arrives mid-process. Also marks + * the event as in-flight so dedup checks in `enqueue` know not to + * fold a fresh content change into an event whose disk read already + * happened. + */ + public peekFront(): SyncEvent | undefined { + return this.events[0]; + } + + /** + * Remove a specific event after `peekFront`-based processing is done. + * Idempotent — safe to call when the event was already taken out by + * `resolveCreate` (which clears a same-path pending create that a + * remote-create handler just absorbed). + */ + public consumeEvent(event: SyncEvent): void { + removeFromArray(this.events, event); + } + + /** * Call once a create has been acknowledged by the server. */ @@ -255,6 +302,42 @@ export class SyncEventQueue { return this.save(); } + /** + * Mark a document as deleted at a given vault-update version. Used by + * the syncer after a successful local or remote delete so future + * obsolete broadcasts for that doc (older parents that arrive late) + * don't resurrect it as a brand-new create. + */ + public recordDeletion( + documentId: DocumentId, + deletedAtVaultUpdateId: VaultUpdateId + ): void { + const existing = this.deletedDocuments.get(documentId); + if (existing !== undefined && existing >= deletedAtVaultUpdateId) { + return; + } + this.deletedDocuments.set(documentId, deletedAtVaultUpdateId); + } + + /** + * Returns the vault-update version at which we last saw this document + * deleted, or `undefined` if we have no record of its deletion. + */ + public getDeletionVersion( + documentId: DocumentId + ): VaultUpdateId | undefined { + return this.deletedDocuments.get(documentId); + } + + /** + * Forget a doc's tombstone — used when a doc with the same id is + * re-introduced (e.g. via a remote create whose server-side state + * surpasses the previous delete). + */ + public clearDeletion(documentId: DocumentId): void { + this.deletedDocuments.delete(documentId); + } + public getDocumentByDocumentId( target: DocumentId ): DocumentWithPath | undefined { @@ -327,6 +410,8 @@ export class SyncEventQueue { ); } + + public async clearAllState(): Promise { this.clearPending(); this.documents.clear(); diff --git a/frontend/sync-client/src/sync-operations/syncer.ts b/frontend/sync-client/src/sync-operations/syncer.ts index dea53285..fd924c15 100644 --- a/frontend/sync-client/src/sync-operations/syncer.ts +++ b/frontend/sync-client/src/sync-operations/syncer.ts @@ -128,7 +128,7 @@ export class Syncer { this.runningScheduleSyncForOfflineChanges = this.internalScheduleSyncForOfflineChanges(); await this.runningScheduleSyncForOfflineChanges; - this.logger.info(`All local changes have been applied remotely`); + this.logger.info(`All local changes have been queued`); } catch (e) { if (e instanceof SyncResetError) { this.logger.info( @@ -192,9 +192,8 @@ export class Syncer { // queue re-enables conflict filtering when we're done. this.queue.setIgnoreConflictPaths(false); try { - while (this.drainPromise !== undefined) { - await this.drainPromise; - } + this.queue.clearPending(); // can't have conflicts between the offline scan and ongoing operations created during the preceeding pause + await scheduleOfflineChanges( this.logger, this.operations, @@ -226,8 +225,21 @@ export class Syncer { } private async drain(): Promise { - let event = await this.queue.next(); - while (event !== undefined) { + // Peek then remove-after-processing (instead of shift-then-process): + // the event must remain reachable through `findLatestCreateForPath` + // while it is in flight, so a rename event arriving mid-process can + // call `updatePendingCreatePath` to retarget this create's path. + while (true) { + if (!this.settings.getSettings().isSyncEnabled) { + this.logger.debug( + "Drain pausing because sync is disabled; events stay queued" + ); + return; + } + const event = this.queue.peekFront(); + + if (event === undefined) { break; } + try { await this.processEvent(event); } catch (e) { @@ -239,19 +251,12 @@ export class Syncer { `Failed to process sync event ${event.type}: ${e}` ); } + this.queue.consumeEvent(event); this.notifyRemainingOperationsChanged(); - event = await this.queue.next(); } } private async processEvent(event: SyncEvent): Promise { - if (!this.settings.getSettings().isSyncEnabled) { - this.logger.info( - `Skipping sync operation because sync is disabled` - ); - return; - } - try { if (await this.skipIfOversized(event)) { return; @@ -272,22 +277,27 @@ export class Syncer { break; } } catch (e) { - // The currently-processed event was already shifted off the queue - // by drain() before processEvent ran. If it's a LocalCreate, any - // queued Delete/Update events whose `documentId` is this Create's - // resolvers.promise would `await` it forever once we return — so - // settle the resolvers on every failure path before - // dispatching/re-throwing. clearPending()'s rejectAllPendingCreates - // walks the queue and so cannot reach this in-flight event. - // Re-rejecting an already-resolved promise is a no-op, so it's - // safe to call this unconditionally on the LocalCreate branch. - if (event.type === SyncEventType.LocalCreate) { + // If a LocalCreate fails terminally, queued LocalDelete / + // LocalUpdate events whose `documentId` is this Create's + // `resolvers.promise` would `await` it forever — reject the + // resolver so they fail-fast with the same error class and + // hit their matching skip/log branch below. + // + // Only do this for terminal errors. `SyncResetError` is + // transient: drain returns without consuming the event, so + // the next drain retries the same Create. Rejecting the + // resolver now would permanently poison it, and the eventual + // `resolveCreate(...resolve)` after the retry succeeds is a + // no-op on an already-settled promise — leaving every + // dependent event stuck failing on `await event.documentId`. + if ( + event.type === SyncEventType.LocalCreate && + !(e instanceof SyncResetError) + ) { event.resolvers.promise.catch(() => { /* suppressed */ }); - event.resolvers.reject( - new Error(`Create was cancelled: ${e}`) - ); + event.resolvers.reject(e); } if (e instanceof FileNotFoundError) { @@ -364,8 +374,7 @@ export class Syncer { private async processCreate( event: Extract ): Promise { - const effectivePath = event.path; - const contentBytes = await this.operations.read(effectivePath); + const contentBytes = await this.operations.read(event.path); const contentHash = await hash(contentBytes); const response = await this.syncService.create({ @@ -375,7 +384,7 @@ export class Syncer { }); await this.handleMaybeMergingResponse({ - path: effectivePath, + path: event.path, response, contentHash, originalContentBytes: contentBytes, @@ -384,7 +393,7 @@ export class Syncer { this.history.addHistoryEntry({ status: SyncStatus.SUCCESS, - details: { type: SyncType.CREATE, relativePath: effectivePath }, + details: { type: SyncType.CREATE, relativePath: event.path }, message: response.type === "MergingUpdate" ? "Created file and merged with existing remote version" @@ -399,7 +408,15 @@ export class Syncer { ): Promise { const documentId = await event.documentId; - const doc = this.queue.getDocumentByDocumentIdOrFail(documentId); + const doc = this.queue.getDocumentByDocumentId(documentId); + if (doc === undefined) { + // Already deleted (e.g. a remote delete drained ahead of + // this redundant local one). Nothing to do. + this.logger.debug( + `Skipping local-delete for ${documentId} — doc no longer tracked` + ); + return; + } const relativePath = doc.path; const response = await this.syncService.delete({ @@ -408,6 +425,7 @@ export class Syncer { }); await this.queue.removeDocument(doc.path); + this.queue.recordDeletion(documentId, response.vaultUpdateId); this.queue.lastSeenUpdateId = response.vaultUpdateId; this.history.addHistoryEntry({ @@ -426,8 +444,17 @@ export class Syncer { ): Promise { const documentId = await event.documentId; - const { path: diskPath, record } = - this.queue.getDocumentByDocumentIdOrFail(documentId); + const tracked = this.queue.getDocumentByDocumentId(documentId); + if (tracked === undefined) { + // The doc was deleted between this event being queued and + // drained — skip silently. Common when a LocalDelete drains + // ahead of a LocalUpdate that was already in the queue. + this.logger.debug( + `Skipping local-update for ${documentId} — doc no longer tracked (deleted)` + ); + return; + } + const { path: diskPath, record } = tracked; const contentBytes = await this.operations.read(diskPath); const contentHash = await hash(contentBytes); @@ -518,18 +545,50 @@ export class Syncer { } if (createEvent === undefined) { - // a http response will always be more up-to-date than any queued remote update - // move will always move to the relative path when MoveOnConflict.EXISTING is given - await this.operations.move( - path, - response.relativePath, - MoveOnConflict.EXISTING + // The disk path captured at the start of `processLocalUpdate` + // can be stale: the user may have renamed the file during the + // server roundtrip, in which case `queue.documents` already + // points at the new path and a follow-up rename's LocalUpdate + // is queued behind us. If we forced the disk back to + // `response.relativePath` here we'd undo the user's intent; + // worse, `setDocument`'s same-docId cleanup would clobber the + // map entry that was tracking the latest disk path, leaving + // future LocalUpdates for this doc reading from a vacated + // slot and getting skipped as `FileNotFoundError`. Refresh + // the latest tracked path and only touch disk when it still + // matches the captured one. + const tracked = this.queue.getDocumentByDocumentId( + response.documentId ); + if (tracked === undefined) { + this.logger.debug( + `Document ${response.documentId} is no longer tracked after update; cannot reconcile potential rename` + ); + } else { + const currentPath = tracked.path ?? path; + if (currentPath === path) { + // a http response will always be more up-to-date than any queued remote update + // move will always move to the relative path when MoveOnConflict.EXISTING is given + await this.operations.move( + path, + response.relativePath, + MoveOnConflict.EXISTING + ); - await this.queue.setDocument(response.relativePath, { - ...record, - remoteHash - }); + await this.queue.setDocument(response.relativePath, { + ...record, + remoteHash + }); + } else { + // User renamed during the roundtrip. Leave the disk file + // at `currentPath`; the queued rename's LocalUpdate will + // reconcile the server on its next drain. + await this.queue.setDocument(currentPath, { + ...record, + remoteHash + }); + } + } } else { // The server may have deconflicted the path on create (e.g. // another client raced us to the same path and won). Move the @@ -574,6 +633,24 @@ export class Syncer { ); } + // The doc was deleted at-or-after the version this broadcast + // describes (e.g. another client's update committed before our + // local delete; the server's backlog is replaying it now). Apply + // would resurrect a doc we deliberately removed. + const deletedAt = this.queue.getDeletionVersion( + remoteVersion.documentId + ); + if ( + deletedAt !== undefined && + deletedAt >= remoteVersion.vaultUpdateId + ) { + this.queue.lastSeenUpdateId = remoteVersion.vaultUpdateId; + this.logger.debug( + `Skipping obsolete remote update for already-deleted document ${remoteVersion.documentId} (V=${remoteVersion.vaultUpdateId} <= deleted V=${deletedAt})` + ); + return; + } + if ( (documentWithPath?.record.parentVersionId ?? 0) >= remoteVersion.vaultUpdateId @@ -598,14 +675,7 @@ export class Syncer { remoteVersion.relativePath ); - if (pendingCreate === undefined) { - return this.processRemoteCreateForNewDocument(remoteVersion); - } else { - return this.processRemoteCreateForPendingDocument( - remoteVersion, - pendingCreate - ); - } + return this.processRemoteCreateForNewDocument(remoteVersion); } private async processRemoteDelete( @@ -614,6 +684,10 @@ export class Syncer { ): Promise { await this.operations.delete(path); await this.queue.removeDocument(path); + this.queue.recordDeletion( + remoteVersion.documentId, + remoteVersion.vaultUpdateId + ); this.queue.lastSeenUpdateId = remoteVersion.vaultUpdateId; @@ -770,53 +844,7 @@ export class Syncer { }); } - // A remote create landed at a path where we have an unsynced local - // create. This might be becuase there's another sync client running. - // We must avoid duplicating files. - private async processRemoteCreateForPendingDocument( - remoteVersion: DocumentVersionWithoutContent, - pendingCreateEvent: Extract< - SyncEvent, - { type: SyncEventType.LocalCreate } - > - ): Promise { - const remoteContent = await this.syncService.getDocumentVersionContent({ - documentId: remoteVersion.documentId, - vaultUpdateId: remoteVersion.vaultUpdateId - }); - const remoteHash = await hash(remoteContent); - const path = remoteVersion.relativePath; - const currentContent = await this.operations.read( - pendingCreateEvent.path - ); - - await this.operations.write(path, currentContent, remoteContent); - await this.updateCache( - remoteVersion.vaultUpdateId, - remoteContent, - path - ); - - await this.queue.resolveCreate(pendingCreateEvent, { - documentId: remoteVersion.documentId, - parentVersionId: remoteVersion.vaultUpdateId, - remoteHash, - remoteRelativePath: path - }); - this.queue.lastSeenUpdateId = remoteVersion.vaultUpdateId; - - this.history.addHistoryEntry({ - status: SyncStatus.SUCCESS, - details: { - type: SyncType.UPDATE, - relativePath: path - }, - message: `Adopted remote create at ${path}`, - author: remoteVersion.userId, - timestamp: new Date(remoteVersion.updatedDate) - }); - } private async sendUpdate({ record, diff --git a/sync-server/src/server/create_document.rs b/sync-server/src/server/create_document.rs index 84703139..8230ea46 100644 --- a/sync-server/src/server/create_document.rs +++ b/sync-server/src/server/create_document.rs @@ -73,7 +73,10 @@ pub async fn create_document( // but client 2 moves it to P2 while client 1 creates a new document at P2, // then client 1 would merge its new document with the moved version of A at P2 // that client 2 resulting in two files (P1 and P2) with the same doc id (A). - if latest_version.creation_vault_update_id > request.last_seen_vault_update_id { + if latest_version.creation_vault_update_id > request.last_seen_vault_update_id + && latest_version.creation_vault_update_id == latest_version.vault_update_id + // can't allow merging with a moved document as that could create a cycle + { let is_mergeable_text = is_file_type_mergable( &sanitized_relative_path, &state.config.server.mergeable_file_extensions,