Skip to content

fix(merge): replace two-way DMP patch with proper three-way merge — prevents silent data loss on conflict (#353, #180) #375

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
iamhyc wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

fix(merge): replace two-way DMP patch with proper three-way merge — prevents silent data loss on conflict (#353, #180) #375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions src/api/socketioAlt.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
/* eslint-disable @typescript-eslint/naming-convention */
import * as vscode from 'vscode';
import * as DiffMatchPatch from 'diff-match-patch';
import { EventEmitter } from 'events';
import { BaseAPI, Identity, ProjectMessageResponseSchema, ProjectSettingsSchema } from './base';
import { DocumentEntity, FileEntity, ProjectEntity, VirtualFileSystem } from '../core/remoteFileSystemProvider';
import { UpdateSchema, UpdateUserSchema } from './socketio';
import { ROOT_NAME } from '../consts';
import { threeWayMerge, tryTrivialMerge } from '../utils/threeWayMerge';

const keyHistoryRefreshInterval = `${ROOT_NAME}.invisibleMode.historyRefreshInterval`;
const keyChatMessageRefreshInterval = `${ROOT_NAME}.invisibleMode.chatMessageRefreshInterval`;
Expand Down Expand Up @@ -211,13 +211,24 @@ export class SocketIOAlt {
if (_doc && _doc.isDirty) { await _doc.save(); }
// generate patch and apply locally
const vfsLocalVersion = this.vfsLocalVersion!;
const dmp = new DiffMatchPatch();
const localContent = new TextDecoder().decode( await vfs.openFile(_uri) );
const baseRemoteContent = (await vfs.getFileDiff(pathname, vfsLocalVersion, vfsLocalVersion))?.diff[0].u;
const latestRemoteContent = (await vfs.getFileDiff(pathname, latestVersion, latestVersion))?.diff[0].u;
if (baseRemoteContent!==undefined && latestRemoteContent!==undefined) {
const patch = dmp.patch_make(baseRemoteContent, latestRemoteContent);
const [localContentPatched, _] = dmp.patch_apply(patch, localContent);
// Perform a proper three-way merge:
// base = baseRemoteContent (common ancestor)
// local = localContent (current local file)
// remote = latestRemoteContent (latest server state)
let localContentPatched = tryTrivialMerge(baseRemoteContent, localContent, latestRemoteContent);
if (localContentPatched === undefined) {
const mergeResult = threeWayMerge(baseRemoteContent, localContent, latestRemoteContent);
localContentPatched = mergeResult.content;
if (mergeResult.hasConflict) {
vscode.window.showWarningMessage(
vscode.l10n.t('Merge conflict in "{0}". Remote changes overlap with your local edits. Conflict markers have been inserted — please review and resolve.', pathname)
);
}
}
this._eventEmitter.emit('otUpdateApplied', {
doc: entityId,
v: (entity as DocumentEntity).version, //bypass update check
Expand Down
58 changes: 54 additions & 4 deletions src/core/remoteFileSystemProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { ClientManager } from '../collaboration/clientManager';
import { EventBus } from '../utils/eventBus';
import { SCMCollectionProvider } from '../scm/scmCollectionProvider';
import { ExtendedBaseAPI, ProjectLinkedFileProvider, UrlLinkedFileProvider } from '../api/extendedBase';
import { threeWayMerge, tryTrivialMerge } from '../utils/threeWayMerge';

const __OUTPUTS_ID = `${ROOT_NAME}-outputs`;

Expand All @@ -34,6 +35,8 @@ export interface DocumentEntity extends FileEntity {
lastVersion?: number,
localCache?: string,
remoteCache?: string,
/** Server state at the moment a conflict was detected. When set, the next writeFile is a post-conflict resolution save; the OT op is computed from this base, then the field is cleared. */
_otBase?: string,
}

export interface FileRefEntity extends FileEntity {
Expand Down Expand Up @@ -745,11 +748,58 @@ export class VirtualFileSystem extends vscode.Disposable {
if (doc.version===undefined || doc.localCache===undefined || doc.remoteCache===undefined) {
return;
}
const dmp = new DiffMatchPatch();
const patches = dmp.patch_make(doc.localCache, doc.remoteCache);

const mergeResArray = dmp.patch_apply(patches, _content);
const mergeRes = mergeResArray[0] as string;
// If _otBase is set, a previous conflict was detected and the user has
// since resolved the markers. Skip merge — compute the OT op from the
// real server state stored in _otBase.
let mergeRes: string;
let hasConflict = false;

if (doc._otBase !== undefined) {
doc.remoteCache = doc._otBase;
delete doc._otBase;
mergeRes = _content;
} else {
// Perform a proper three-way merge (like Git's diff3) using:
// base = doc.localCache (what we last agreed on locally)
// local = _content (user's current edits)
// remote = doc.remoteCache (latest known server state)
const trivial = tryTrivialMerge(doc.localCache, _content, doc.remoteCache);
if (trivial !== undefined) {
mergeRes = trivial;
} else {
const mergeResult = threeWayMerge(doc.localCache, _content, doc.remoteCache);
mergeRes = mergeResult.content;
hasConflict = mergeResult.hasConflict;
}
}

if (hasConflict) {
// Conflicts detected: write conflict markers to disk so VS Code's
// built-in merge conflict UI can help the user resolve them.
// Do NOT send an OT update to the server while conflicts exist.
// Preserve the server state in _otBase so the post-resolution
// save can compute the OT op from the correct base.
const conflictContent = new TextEncoder().encode(mergeRes);
await vscode.workspace.fs.writeFile(uri, conflictContent);
doc._otBase = doc.remoteCache;
doc.localCache = mergeRes;
this.isDirty = true;

vscode.window.showWarningMessage(
vscode.l10n.t('Merge conflict detected in "{0}". Your changes overlap with changes from the server. Please review the conflict markers in the editor and save after resolving.', doc.name)
);

setTimeout(() => {
this.notify([
{type: vscode.FileChangeType.Changed, uri: uri}
]);
}, 10);
return;
}

// No conflict: proceed with the merged content
const dmp = new DiffMatchPatch();
const update = {
doc: doc._id,
lastV: doc.lastVersion,
Expand Down
47 changes: 37 additions & 10 deletions src/scm/localReplicaSCM.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as DiffMatchPatch from 'diff-match-patch';
import { minimatch } from 'minimatch';
import { BaseSCM, CommitItem, SettingItem } from ".";
import { VirtualFileSystem, parseUri } from '../core/remoteFileSystemProvider';
import { threeWayMerge, tryTrivialMerge } from '../utils/threeWayMerge';

const IGNORE_SETTING_KEY = 'ignore-patterns';

Expand Down Expand Up @@ -249,20 +250,46 @@ export class LocalReplicaSCMProvider extends BaseSCM {
this.setBypassCache(relPath, remoteContent);
await this.writeFile(relPath, remoteContent);
} else {
const dmp = new DiffMatchPatch();
const baseContentStr = new TextDecoder().decode(baseContent);
const localContentStr = new TextDecoder().decode(localContent);
const remoteContentStr = new TextDecoder().decode(remoteContent);
// merge local and remote changes
const localPatches = dmp.patch_make( baseContentStr, localContentStr );
const remotePatches = dmp.patch_make( baseContentStr, remoteContentStr );
const [mergedContentStr, _results] = dmp.patch_apply( remotePatches, localContentStr );
// write the merged content to local

// Perform a proper three-way merge (like Git's diff3):
// base = last known common state
// local = current local file content
// remote = current remote (VFS) file content
//
// First try trivial merges:
let mergedContentStr = tryTrivialMerge(baseContentStr, localContentStr, remoteContentStr);
let hasConflict = false;

if (mergedContentStr === undefined) {
// Need full three-way merge
const mergeResult = threeWayMerge(baseContentStr, localContentStr, remoteContentStr);
mergedContentStr = mergeResult.content;
hasConflict = mergeResult.hasConflict;
}

const mergedContent = new TextEncoder().encode(mergedContentStr);
await this.writeFile(relPath, mergedContent);
// write the merged content to remote
if (localPatches.length!==0) {
await vscode.workspace.fs.writeFile(vfsUri, mergedContent);

if (hasConflict) {
// Write conflicted content with markers to local only.
// Do NOT push to remote — the user must resolve conflicts first.
await this.writeFile(relPath, mergedContent);
this.setBypassCache(relPath, mergedContent);
vscode.window.showWarningMessage(
vscode.l10n.t('Merge conflict in "{0}". Local and remote changes overlap. Conflict markers have been inserted — please review and resolve, then save.', relPath)
);
} else {
// No conflict: write merged content to local
await this.writeFile(relPath, mergedContent);
// Push merged content to remote if local had changes
const dmp = new DiffMatchPatch();
const localPatches = dmp.patch_make( baseContentStr, localContentStr );
if (localPatches.length!==0) {
await vscode.workspace.fs.writeFile(vfsUri, mergedContent);
}
this.setBypassCache(relPath, mergedContent);
}
}
}
Expand Down
Loading
Loading