diff --git a/docs/specs_conflict_resolution.md b/docs/specs_conflict_resolution.md index 0cfc8f26..569105c6 100644 --- a/docs/specs_conflict_resolution.md +++ b/docs/specs_conflict_resolution.md @@ -25,6 +25,7 @@ Resolving a conflict writes the selected or merged result on one observed branch - A receiving Vault file which exactly matches any available revision in the document tree is treated as previously synchronised content. This includes an ancestor below a deleted losing leaf. - A receiving Vault file whose bytes do not match any available revision is preserved as an unsynchronised local change. - File bytes, rather than path, size, modification time, or revision generation, determine whether content is known. +- Three or more live versions are reviewed one pair at a time in a deterministic order, with each completed pair committed before the next live pair is read. - Each device records the exact revision most recently reflected in each Vault file. An edit, deletion, or case-only rename made while a conflict is active extends that displayed branch rather than the deterministic database winner. - A cross-path rename stores the target before logically deleting only the displayed source branch. @@ -72,6 +73,16 @@ A device can resolve only the leaves which it has observed. If another device ha A higher revision generation or modification time does not make either result authoritative. The resolver must examine every current live leaf again until one result remains or user action is required. This is continued conflict processing, not a reset of the synchronisation checkpoint. +## More than two live versions + +When three or more versions remain, LiveSync compares the current PouchDB winner with one conflict leaf at a time. Commonlib orders the remaining candidates by revision generation ascending, original leaf modification time ascending, then the complete revision ID in code-unit lexical order. A missing or non-finite modification time is ordered before a finite value. Modification time makes pair selection reproducible here; it does not decide which content wins. + +For each pair, LiveSync first collapses identical content, then attempts a conservative sensible merge, and finally asks the user when neither automatic action is safe. A completed action is written to the ordinary revision tree and its losing observed leaf is deleted before LiveSync reads the remaining live leaves again. There is no separate persistent merge accumulator. + +**Concat both** writes the concatenated result as a new child of the displayed PouchDB winner, then deletes only the other leaf shown in that dialogue. With two live versions, that action resolves the conflict. With three or more, the new child remains live against every untouched leaf and becomes part of the next pairwise review; it does not create an unrelated root or consume an unseen branch. + +Consequently, choosing **Not now** or closing Obsidian cannot undo a completed pair. After restart, LiveSync reconstructs the next pair from the live tree. If replication changes either revision while a dialogue is open, LiveSync discards the stale selection, refreshes the live count, and rechecks the path rather than deleting a revision which was not the one shown. + ## Device-local file provenance LiveSync composes Commonlib's injected `FileReflectionProvenance` with its local key-value database. Each device stores: @@ -102,24 +113,34 @@ If an edit's base cannot be proved, LiveSync keeps the bytes as another manual-r Choosing **Not now** postpones repeated merge dialogues for the same uninterrupted conflict episode in the current plug-in session. Ordinary file checks and replication do not reopen the dialogue while at least one conflict -leaf remains. If the in-editor status display is enabled, the active file keeps -the warning **This file has unresolved conflicts.** so that postponement does -not make the conflict invisible. +leaf remains. If the in-editor status display is enabled, the active file shows +**This file has 3 unresolved versions. They will be reviewed one pair at a +time.** for three or more live versions, using the current count, and **This +file has unresolved conflicts.** for two. Postponement therefore does not make +the conflict invisible. The command **Resolve if conflicted.**, and selecting a file through **Pick a file to resolve conflict**, explicitly clear the postponement and request the dialogue again. Cancellation caused by another conflict dialogue does not count as **Not now**. Once the document has no remaining conflicts, the episode ends; a later conflict at the same path prompts normally. The postponement is not -persisted across a plug-in reload. +persisted across a plug-in reload. Completed pairwise resolutions are persisted +in the ordinary revision tree, so a reload forgets only the postponement and +does not repeat an already committed stage. When synchronisation supplies a resolved document, the existing incoming-file processing event closes an open conflict dialogue for that path. The same event rechecks the local revision tree: if no conflict leaf remains, it ends any -postponed episode and removes the active-file warning. If another conflict leaf -still exists, the stale dialogue closes, but the warning remains. A postponed -episode stays postponed; otherwise, subsequent conflict processing may open a -fresh dialogue for the current revision tree. +postponed episode and removes the active-file warning. If conflict leaves still +exist, the stale dialogue closes and the warning changes to the current live +version count. A postponed episode stays postponed; otherwise, subsequent +conflict processing may open a fresh dialogue for the current revision tree. +Each dialogue owns its completion result, so a prompt which is answered or +closed immediately still completes the waiting conflict operation; the result +does not depend on a later global listener being ready. +The end of an automatic repeat is silent. An explicit **Pick a file to resolve +conflict** request which starts with no conflicts may show one confirmation +Notice. ## Example device scenarios @@ -167,6 +188,12 @@ Android may resolve a conflict and continue editing while Mac still shows the lo If the user edited the file on Mac before the resolution arrived, the bytes no longer match that historical revision. LiveSync preserves the Mac edit as an unsynchronised conflict instead of overwriting it. +### A three-version review is interrupted + +Mac receives three live versions of `shared.md`. The active-file status reports three unresolved versions, and the first dialogue compares the deterministic winner with the first ordered conflict leaf. The user completes that pair, leaving two live versions, then chooses **Not now** on the next dialogue and closes Obsidian. + +The first decision has already changed the ordinary revision tree. On restart, LiveSync reads the two surviving versions and presents only that remaining pair; it does not reconstruct the original three-version state. If another device resolves the remaining pair before or while the dialogue is open, the warning disappears and the stale dialogue closes. + ### The device-local record is missing A local-database reset removes revision provenance. On the next scan, if the Vault file matches exactly one available revision, LiveSync can reconstruct which branch was displayed and continue from it. If the bytes match multiple revisions, or no available revision, the branch remains unproved. @@ -193,6 +220,8 @@ Do not: ## Verification -Commonlib's real-PouchDB and injected-boundary unit tests cover unequal branch lengths, exact shared ancestry, content below a deleted losing leaf, recorded and reconstructed branch identity, ambiguous matches, conflict-time editing, logical deletion, case-only rename, cross-path rename, and safe unproven fallbacks. +Commonlib's real-PouchDB and injected-boundary unit tests cover unequal branch lengths, exact shared ancestry, deterministic ordering of multiple live leaves, a sensible stage followed by reconstruction of a manual pair, content below a deleted losing leaf, recorded and reconstructed branch identity, ambiguous matches, conflict-time editing, logical deletion, case-only rename, cross-path rename, and safe unproven fallbacks. LiveSync's optional real-Obsidian two-Vault checks have two scopes. `E2E_OBSIDIAN_INCLUDE_MARKDOWN_CONFLICT=true` resolves and edits a Markdown conflict, propagates it to a Vault which still displays the deleted losing content, and requires one live result to remain. `E2E_OBSIDIAN_INCLUDE_CONFLICT_OPERATIONS=true` edits, deletes, case-renames, and cross-path-renames files while conflicts remain active; it verifies the parent revision of each resulting branch, replicates those exact trees, and confirms that the other live branches remain intact. + +The focused `test:e2e:obsidian:conflict-dialog-policy` scenario creates three live versions in one real Obsidian Vault. It verifies the count warning, commits a concatenated child of the displayed winner, confirms that the untouched leaf remains as one conflict, postpones that remaining pair, restarts the isolated Obsidian profile, and confirms that only the live pair is reconstructed. It also verifies that an incoming resolution closes a stale dialogue, completes the waiting conflict operation, and clears the warning. diff --git a/package-lock.json b/package-lock.json index 1f2c1687..b48f5434 100644 --- a/package-lock.json +++ b/package-lock.json @@ -22,7 +22,7 @@ "@smithy/querystring-builder": "^4.2.9", "@smithy/types": "^4.14.3", "@smithy/util-retry": "^4.4.5", - "@vrtmrz/livesync-commonlib": "0.1.0-rc.10", + "@vrtmrz/livesync-commonlib": "0.1.0-rc.11", "@vrtmrz/obsidian-plugin-kit": "0.1.2", "diff-match-patch": "^1.0.5", "fflate": "^0.8.2", @@ -4898,9 +4898,9 @@ } }, "node_modules/@vrtmrz/livesync-commonlib": { - "version": "0.1.0-rc.10", - "resolved": "https://registry.npmjs.org/@vrtmrz/livesync-commonlib/-/livesync-commonlib-0.1.0-rc.10.tgz", - "integrity": "sha512-LFhDfIwXl5lPD92/hhQpD8aTlgOakT0yb9vZ4TcKEeTRyc+Os7eGUB93LcKdZhYC9fKTh/tdyfd0g2NJDMlKkA==", + "version": "0.1.0-rc.11", + "resolved": "https://registry.npmjs.org/@vrtmrz/livesync-commonlib/-/livesync-commonlib-0.1.0-rc.11.tgz", + "integrity": "sha512-o811duZFajxDFI6cy7zwtXPg6lihx5e1MH6Xse1sx4HseYurbaDf9DtZJdAtfkjTxl5wOZRSnVMoMLn+I/xD+g==", "license": "MIT", "dependencies": { "@aws-sdk/client-s3": "^3.808.0", diff --git a/package.json b/package.json index b047ca52..42c21c94 100644 --- a/package.json +++ b/package.json @@ -163,7 +163,7 @@ "@smithy/querystring-builder": "^4.2.9", "@smithy/types": "^4.14.3", "@smithy/util-retry": "^4.4.5", - "@vrtmrz/livesync-commonlib": "0.1.0-rc.10", + "@vrtmrz/livesync-commonlib": "0.1.0-rc.11", "@vrtmrz/obsidian-plugin-kit": "0.1.2", "diff-match-patch": "^1.0.5", "fflate": "^0.8.2", diff --git a/src/common/messages/LiveSyncProvisionalMessages.ts b/src/common/messages/LiveSyncProvisionalMessages.ts index 0b8bd745..2ec64b67 100644 --- a/src/common/messages/LiveSyncProvisionalMessages.ts +++ b/src/common/messages/LiveSyncProvisionalMessages.ts @@ -8,6 +8,8 @@ */ export const liveSyncProvisionalEnglishMessages = { "This file has unresolved conflicts.": "This file has unresolved conflicts.", + "This file has ${COUNT} unresolved versions. They will be reviewed one pair at a time.": + "This file has ${COUNT} unresolved versions. They will be reviewed one pair at a time.", } as const; export type LiveSyncProvisionalMessageKey = keyof typeof liveSyncProvisionalEnglishMessages; diff --git a/src/common/translation.ts b/src/common/translation.ts index ff7e742d..1b85bb91 100644 --- a/src/common/translation.ts +++ b/src/common/translation.ts @@ -143,6 +143,6 @@ export function $msg( export type LiveSyncMessageTranslator = MessageTranslator; export const translateLiveSyncMessage: LiveSyncMessageTranslator = (key, params) => { - if (!isLiveSyncMessageKey(key)) return englishMessageTranslator(key as CommonlibMessageKey, params); + if (!isLiveSyncMessageKey(key)) return englishMessageTranslator(key, params); return $msg(key, params === undefined ? undefined : { ...params }); }; diff --git a/src/common/translation.unit.spec.ts b/src/common/translation.unit.spec.ts index 30fdd1a6..da736044 100644 --- a/src/common/translation.unit.spec.ts +++ b/src/common/translation.unit.spec.ts @@ -28,6 +28,11 @@ describe("LiveSync-owned translation catalogue", () => { it("uses LiveSync-owned provisional English without extending Commonlib's message contract", () => { expect($msg("This file has unresolved conflicts.")).toBe("This file has unresolved conflicts."); + expect( + $msg("This file has ${COUNT} unresolved versions. They will be reviewed one pair at a time.", { + COUNT: "3", + }) + ).toBe("This file has 3 unresolved versions. They will be reviewed one pair at a time."); expect(translateLiveSyncMessage("This file has unresolved conflicts.")).toBe( "This file has unresolved conflicts." ); diff --git a/src/modules/coreFeatures/ModuleConflictResolver.unit.spec.ts b/src/modules/coreFeatures/ModuleConflictResolver.unit.spec.ts index b3c08ee3..c1348c43 100644 --- a/src/modules/coreFeatures/ModuleConflictResolver.unit.spec.ts +++ b/src/modules/coreFeatures/ModuleConflictResolver.unit.spec.ts @@ -12,6 +12,8 @@ import { ModuleConflictResolver } from "./ModuleConflictResolver"; function createModule(files: FilePathWithPrefix[] = []) { const resolveByDeletingRevision = vi.fn(async () => AUTO_MERGED); const tryAutoMerge = vi.fn(); + const queueCheckFor = vi.fn(async () => undefined); + const resolveByUserInteraction = vi.fn(async () => false); const core = { _services: { API: { @@ -27,6 +29,17 @@ function createModule(files: FilePathWithPrefix[] = []) { conflict: { resolveByNewest: vi.fn(async () => true), resolveByDeletingRevision, + resolveByUserInteraction, + queueCheckFor, + }, + appLifecycle: { + isSuspended: vi.fn(() => false), + }, + replication: { + replicateByEvent: vi.fn(async () => true), + }, + vault: { + getActiveFilePath: vi.fn(() => undefined), }, }, settings: DEFAULT_SETTINGS, @@ -49,7 +62,7 @@ function createModule(files: FilePathWithPrefix[] = []) { const module = new ModuleConflictResolver(core); module._log = vi.fn(); - return { module, resolveByDeletingRevision, tryAutoMerge }; + return { module, queueCheckFor, resolveByDeletingRevision, resolveByUserInteraction, tryAutoMerge }; } describe("ModuleConflictResolver bulk newest resolution", () => { @@ -193,4 +206,40 @@ describe("ModuleConflictResolver sensible merge hand-off", () => { ); expect(resolveByDeletingRevision).toHaveBeenCalledWith(path, "2-right", "Sensible"); }); + + it("commits a sensible pair before rechecking the remaining manual pair", async () => { + const path = "three-versions.md" as FilePathWithPrefix; + const { module, queueCheckFor, resolveByDeletingRevision, resolveByUserInteraction, tryAutoMerge } = + createModule(); + const remainingManualPair = { + leftRev: "3-merged", + rightRev: "2-third", + leftLeaf: { rev: "3-merged", data: "Merged\n", ctime: 1, mtime: 3 }, + rightLeaf: { rev: "2-third", data: "Overlapping\n", ctime: 1, mtime: 2 }, + }; + tryAutoMerge + .mockResolvedValueOnce({ + result: "Merged\n", + conflictedRev: "2-second", + }) + .mockResolvedValueOnce(remainingManualPair); + + await (module as any)._resolveConflict(path); + + expect(module.core.databaseFileAccess.storeContent).toHaveBeenCalledWith(path, "Merged\n"); + expect(resolveByDeletingRevision).toHaveBeenCalledWith(path, "2-second", "Sensible"); + expect(queueCheckFor).toHaveBeenCalledWith(path); + expect(resolveByUserInteraction).not.toHaveBeenCalled(); + + await (module as any)._resolveConflict(path); + + expect(tryAutoMerge).toHaveBeenCalledTimes(2); + expect(resolveByUserInteraction).toHaveBeenCalledWith( + path, + expect.objectContaining({ + left: remainingManualPair.leftLeaf, + right: remainingManualPair.rightLeaf, + }) + ); + }); }); diff --git a/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts index 0ca07d90..8e53f4a9 100644 --- a/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts +++ b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.ts @@ -1,27 +1,26 @@ import { App, Modal } from "@/deps.ts"; import { DIFF_DELETE, DIFF_EQUAL, DIFF_INSERT } from "diff-match-patch"; -import { CANCELLED, LEAVE_TO_SUBSEQUENT, type diff_result } from "@vrtmrz/livesync-commonlib/compat/common/types"; -import { delay } from "@vrtmrz/livesync-commonlib/compat/common/utils"; +import { + CANCELLED, + LEAVE_TO_SUBSEQUENT, + type diff_result, + type FilePathWithPrefix, +} from "@vrtmrz/livesync-commonlib/compat/common/types"; import { EVENT_CONFLICT_CANCELLED, eventHub } from "@/common/events.ts"; -import { globalSlipBoard } from "@vrtmrz/livesync-commonlib/compat/bureau/bureau"; +import { promiseWithResolvers } from "octagonal-wheels/promises"; export const POSTPONED = Symbol("postponed"); export type MergeDialogResult = typeof CANCELLED | typeof POSTPONED | typeof LEAVE_TO_SUBSEQUENT | string; -declare global { - interface Slips { - "conflict-resolved": typeof CANCELLED | MergeDialogResult; - } -} - export class ConflictResolveModal extends Modal { result: diff_result; - filename: string; + filename: FilePathWithPrefix; response: MergeDialogResult = CANCELLED; isClosed = false; consumed = false; + private readonly resultPromise = promiseWithResolvers(); title: string = "Conflicting changes"; @@ -33,7 +32,13 @@ export class ConflictResolveModal extends Modal { diffView!: HTMLDivElement; diffNavIndicator!: HTMLSpanElement; - constructor(app: App, filename: string, diff: diff_result, pluginPickMode?: boolean, remoteName?: string) { + constructor( + app: App, + filename: FilePathWithPrefix, + diff: diff_result, + pluginPickMode?: boolean, + remoteName?: string + ) { super(app); this.result = diff; this.filename = filename; @@ -43,9 +48,6 @@ export class ConflictResolveModal extends Modal { this.remoteName = `${remoteName || "Remote"}`; this.localName = "Local"; } - // Send cancel signal for the previous merge dialogue - // if not there, simply be ignored. - // sendValue("close-resolve-conflict:" + this.filename, false); } appendDiffFragment(container: HTMLDivElement, text: string, cls: string) { @@ -96,18 +98,19 @@ export class ConflictResolveModal extends Modal { override onOpen() { const { contentEl } = this; - // Send cancel signal for the previous merge dialogue - // if not there, simply be ignored. - globalSlipBoard.submit("conflict-resolved", this.filename, CANCELLED); if (this.offEvent) { this.offEvent(); } + // Cancel an older dialogue for this path before subscribing this + // instance. Emitting after subscription would close the replacement + // itself; the instance-owned result promise then completes the older + // caller even when it only begins waiting after this event. + eventHub.emitEvent(EVENT_CONFLICT_CANCELLED, this.filename); this.offEvent = eventHub.onEvent(EVENT_CONFLICT_CANCELLED, (path) => { if (path === this.filename) { this.sendResponse(CANCELLED); } }); - // sendValue("close-resolve-conflict:" + this.filename, false); this.titleEl.setText(this.title); contentEl.empty(); const diffOptionsRow = contentEl.createDiv(""); @@ -155,21 +158,22 @@ export class ConflictResolveModal extends Modal { new Date(this.result.right.mtime).toLocaleString() + (this.result.right.deleted ? " (Deleted)" : ""); this.appendVersionInfo(div2, "deleted", this.localName, date1); this.appendVersionInfo(div2, "added", this.remoteName, date2); - contentEl.createEl("button", { text: `Use ${this.localName}` }, (e) => { + const actionContainer = contentEl.createDiv("conflict-action-container"); + actionContainer.createEl("button", { text: `Use ${this.localName}` }, (e) => { e.addClass("conflict-action-button"); e.addEventListener("click", () => this.sendResponse(this.result.right.rev)); }); - contentEl.createEl("button", { text: `Use ${this.remoteName}` }, (e) => { + actionContainer.createEl("button", { text: `Use ${this.remoteName}` }, (e) => { e.addClass("conflict-action-button"); e.addEventListener("click", () => this.sendResponse(this.result.left.rev)); }); if (!this.pluginPickMode) { - contentEl.createEl("button", { text: "Concat both" }, (e) => { + actionContainer.createEl("button", { text: "Concat both" }, (e) => { e.addClass("conflict-action-button"); e.addEventListener("click", () => this.sendResponse(LEAVE_TO_SUBSEQUENT)); }); } - contentEl.createEl("button", { text: !this.pluginPickMode ? "Not now" : "Cancel" }, (e) => { + actionContainer.createEl("button", { text: !this.pluginPickMode ? "Not now" : "Cancel" }, (e) => { e.addClass("conflict-action-button"); e.addEventListener("click", () => this.sendResponse(this.pluginPickMode ? CANCELLED : POSTPONED)); }); @@ -196,12 +200,10 @@ export class ConflictResolveModal extends Modal { return; } this.consumed = true; - globalSlipBoard.submit("conflict-resolved", this.filename, this.response); + this.resultPromise.resolve(this.response); } async waitForResult(): Promise { - await delay(100); - const r = await globalSlipBoard.awaitNext("conflict-resolved", this.filename); - return r; + return await this.resultPromise.promise; } } diff --git a/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.unit.spec.ts b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.unit.spec.ts new file mode 100644 index 00000000..aaf154d7 --- /dev/null +++ b/src/modules/features/InteractiveConflictResolving/ConflictResolveModal.unit.spec.ts @@ -0,0 +1,85 @@ +import { describe, expect, it, vi } from "vitest"; +import { POSTPONED, ConflictResolveModal } from "./ConflictResolveModal.ts"; +import { CANCELLED, type diff_result, type FilePathWithPrefix } from "@vrtmrz/livesync-commonlib/compat/common/types"; + +vi.mock("@/deps.ts", () => ({ + App: class App {}, + Modal: class Modal { + private createElement(): Record { + const element: Record = { + addClass: vi.fn(), + addEventListener: vi.fn(), + appendText: vi.fn(), + classList: { + add: vi.fn(), + remove: vi.fn(), + }, + empty: vi.fn(), + querySelector: vi.fn(() => null), + querySelectorAll: vi.fn(() => []), + scrollIntoView: vi.fn(), + setText: vi.fn(), + }; + element.createDiv = vi.fn(() => this.createElement()); + element.createEl = vi.fn((_tag: string, _options?: unknown, callback?: (child: unknown) => void) => { + const child = this.createElement(); + callback?.(child); + return child; + }); + element.createSpan = vi.fn(() => this.createElement()); + return element; + } + + contentEl = this.createElement(); + titleEl = { + setText: vi.fn(), + }; + + close() { + (this as { onClose?: () => void }).onClose?.(); + } + }, +})); + +const conflict: diff_result = { + left: { rev: "2-left", data: "left", ctime: 1, mtime: 2 }, + right: { rev: "2-right", data: "right", ctime: 1, mtime: 2 }, + diff: [], +}; + +describe("ConflictResolveModal result lifecycle", () => { + it("returns a response which closes the dialogue before the caller begins waiting", async () => { + const modal = new ConflictResolveModal({} as never, "early-response.md" as FilePathWithPrefix, conflict); + + modal.sendResponse(POSTPONED); + const result = await Promise.race([ + modal.waitForResult(), + new Promise<"timed-out">((resolve) => setTimeout(() => resolve("timed-out"), 250)), + ]); + + expect(result).toBe(POSTPONED); + }); + + it("cancels the previous same-path dialogue without cancelling the replacement", async () => { + const filename = "same-path.md" as FilePathWithPrefix; + const previous = new ConflictResolveModal({} as never, filename, conflict); + const replacement = new ConflictResolveModal({} as never, filename, conflict); + previous.onOpen(); + + replacement.onOpen(); + const previousResult = await Promise.race([ + previous.waitForResult(), + new Promise<"timed-out">((resolve) => setTimeout(() => resolve("timed-out"), 250)), + ]); + const replacementState = await Promise.race([ + replacement.waitForResult(), + new Promise<"still-open">((resolve) => setTimeout(() => resolve("still-open"), 25)), + ]); + + previous.sendResponse(CANCELLED); + replacement.sendResponse(CANCELLED); + + expect(previousResult).toBe(CANCELLED); + expect(replacementState).toBe("still-open"); + }); +}); diff --git a/src/modules/features/ModuleInteractiveConflictResolver.ts b/src/modules/features/ModuleInteractiveConflictResolver.ts index e2e568cc..089aa625 100644 --- a/src/modules/features/ModuleInteractiveConflictResolver.ts +++ b/src/modules/features/ModuleInteractiveConflictResolver.ts @@ -22,9 +22,10 @@ import type { Editor, MarkdownFileInfo, MarkdownView } from "@/deps.ts"; export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { private postponedConflictEpisodes = new Set(); - private async isConflicted(filename: FilePathWithPrefix): Promise { + private async getConflictVersionCount(filename: FilePathWithPrefix): Promise { try { - return (await this.core.databaseFileAccess.getConflictedRevs(filename)).length > 0; + const conflictCount = (await this.core.databaseFileAccess.getConflictedRevs(filename)).length; + return conflictCount === 0 ? 0 : conflictCount + 1; } catch (error) { this._log(`Could not inspect the conflict state of ${filename}`, LOG_LEVEL_VERBOSE); this._log(error, LOG_LEVEL_VERBOSE); @@ -35,19 +36,26 @@ export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { private async getActiveConflictMessages(): Promise { const filename = this.services.vault.getActiveFilePath(); if (!filename) return []; - const conflicted = await this.isConflicted(filename); - if (conflicted === false) { + const versionCount = await this.getConflictVersionCount(filename); + if (versionCount === 0) { this.postponedConflictEpisodes.delete(filename); return []; } - if (conflicted === true || this.postponedConflictEpisodes.has(filename)) { + if (versionCount !== undefined && versionCount >= 3) { + return [ + $msg("This file has ${COUNT} unresolved versions. They will be reviewed one pair at a time.", { + COUNT: `${versionCount}`, + }), + ]; + } + if (versionCount === 2 || this.postponedConflictEpisodes.has(filename)) { return [$msg("This file has unresolved conflicts.")]; } return []; } private async refreshConflictState(filename: FilePathWithPrefix): Promise { - if ((await this.isConflicted(filename)) === false) { + if ((await this.getConflictVersionCount(filename)) === 0) { this.postponedConflictEpisodes.delete(filename); } eventHub.emitEvent(EVENT_ON_UNRESOLVED_ERROR); @@ -115,8 +123,21 @@ export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { this._log(`Merge: Could not read ${filename} from the local database`, LOG_LEVEL_VERBOSE); return false; } - if (!testDoc._conflicts) { + if (!testDoc._conflicts || testDoc._conflicts.length === 0) { this._log(`Merge: Nothing to do ${filename}`, LOG_LEVEL_VERBOSE); + await this.refreshConflictState(filename); + return false; + } + if ( + testDoc._rev !== conflictCheckResult.left.rev || + !testDoc._conflicts.includes(conflictCheckResult.right.rev) + ) { + this._log( + `Merge: The compared revisions changed while the dialogue was open: ${filename}`, + LOG_LEVEL_INFO + ); + await this.refreshConflictState(filename); + await this.services.conflict.queueCheckFor(filename); return false; } const toDelete = selected; @@ -125,7 +146,7 @@ export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { // Concatenate both conflicted revisions. // Create a new file by concatenating both conflicted revisions. const p = conflictCheckResult.diff.map((e) => e[1]).join(""); - const delRev = testDoc._conflicts[0]; + const delRev = conflictCheckResult.right.rev; if (!(await this.core.databaseFileAccess.storeContent(filename, p))) { this._log(`Concatenated content cannot be stored:${filename}`, LOG_LEVEL_NOTICE); return false; @@ -141,7 +162,10 @@ export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { ); return false; } - } else if (typeof toDelete === "string") { + } else if ( + typeof toDelete === "string" && + (toDelete === conflictCheckResult.left.rev || toDelete === conflictCheckResult.right.rev) + ) { // Select one of the conflicted revision to delete. if ( (await this.services.conflict.resolveByDeletingRevision(filename, toDelete, "UI Selected")) == @@ -151,7 +175,7 @@ export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { return false; } } else { - this._log(`Merge: Something went wrong: ${filename}, (${toDelete as string})`, LOG_LEVEL_NOTICE); + this._log(`Merge: Something went wrong: ${filename}, (${String(toDelete)})`, LOG_LEVEL_NOTICE); return false; } // In here, some merge has been processed. @@ -166,10 +190,13 @@ export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { }); } async allConflictCheck() { - while (await this.pickFileForResolve()); + let notifyIfEmpty = true; + while (await this.pickFileForResolve(notifyIfEmpty)) { + notifyIfEmpty = false; + } } - async pickFileForResolve() { + async pickFileForResolve(notifyIfEmpty = true) { const notes: { id: DocumentID; path: FilePathWithPrefix; dispPath: string; mtime: number }[] = []; for await (const doc of this.localDatabase.findAllDocs({ conflicts: true })) { if (!("_conflicts" in doc)) continue; @@ -183,7 +210,9 @@ export class ModuleInteractiveConflictResolver extends AbstractObsidianModule { notes.sort((a, b) => b.mtime - a.mtime); const notesList = notes.map((e) => e.dispPath); if (notesList.length == 0) { - this._log("There are no conflicted documents", LOG_LEVEL_NOTICE); + if (notifyIfEmpty) { + this._log("There are no conflicted documents", LOG_LEVEL_NOTICE); + } return false; } const target = await this.core.confirm.askSelectString("File to resolve conflict", notesList); diff --git a/src/modules/features/ModuleInteractiveConflictResolver.unit.spec.ts b/src/modules/features/ModuleInteractiveConflictResolver.unit.spec.ts index 5ad25058..82c2a8ef 100644 --- a/src/modules/features/ModuleInteractiveConflictResolver.unit.spec.ts +++ b/src/modules/features/ModuleInteractiveConflictResolver.unit.spec.ts @@ -1,7 +1,10 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import { + AUTO_MERGED, CANCELLED, DEFAULT_SETTINGS, + LEAVE_TO_SUBSEQUENT, + LOG_LEVEL_NOTICE, type FilePathWithPrefix, type diff_result, } from "@vrtmrz/livesync-commonlib/compat/common/types"; @@ -40,6 +43,12 @@ const conflict: diff_result = { diff: [], }; +async function* documents(items: unknown[]) { + for (const item of items) { + yield item; + } +} + function createModule(conflictedRevisions: string[] = ["2-right"]) { const handlers = { unresolvedMessages: undefined as undefined | (() => Promise), @@ -64,24 +73,28 @@ function createModule(conflictedRevisions: string[] = ["2-right"]) { }, conflict: { resolveByUserInteraction: { addHandler: vi.fn() }, - resolveByDeletingRevision: vi.fn(), + resolveByDeletingRevision: vi.fn(async () => AUTO_MERGED), queueCheckFor: vi.fn(async () => undefined), ensureAllProcessed: vi.fn(async () => true), }, replication: { replicateByEvent: vi.fn(async () => true) }, vault: { getActiveFilePath: vi.fn(() => path) }, - path: { getPath: vi.fn(), getPathWithoutPrefix: vi.fn() }, + path: { getPath: vi.fn((entry: { path: FilePathWithPrefix }) => entry.path) }, }; const core = { _services: services, services, settings: { ...DEFAULT_SETTINGS, syncAfterMerge: false }, localDatabase: { - getDBEntry: vi.fn(async () => false), - findAllDocs: vi.fn(), + getDBEntry: vi.fn(async (): Promise => false), + findAllDocs: vi.fn(() => documents([])), }, databaseFileAccess: { getConflictedRevs: vi.fn(async () => conflictedRevisions), + storeContent: vi.fn(async () => true), + }, + confirm: { + askSelectString: vi.fn(async (): Promise => undefined), }, }; const plugin = { app: {} }; @@ -158,4 +171,111 @@ describe("ModuleInteractiveConflictResolver postponement", () => { conflictedRevisions.splice(0); await expect(handlers.unresolvedMessages?.()).resolves.toEqual([]); }); + + it("reports the number of live versions and reduces it after each resolved pair", async () => { + const conflictedRevisions = ["2-second", "2-third"]; + const { core, handlers, module, services } = createModule(conflictedRevisions); + module.onBindFunction(core as never, services as never); + + await expect(handlers.unresolvedMessages?.()).resolves.toEqual([ + "This file has 3 unresolved versions. They will be reviewed one pair at a time.", + ]); + + conflictedRevisions.shift(); + await (module as any).refreshConflictState(path); + await expect(handlers.unresolvedMessages?.()).resolves.toEqual(["This file has unresolved conflicts."]); + + conflictedRevisions.shift(); + await (module as any).refreshConflictState(path); + await expect(handlers.unresolvedMessages?.()).resolves.toEqual([]); + }); + + it("reconstructs the remaining pair after a postponed session is restarted", async () => { + const conflictedRevisions = ["2-second", "2-third"]; + const firstSession = createModule(conflictedRevisions); + + await firstSession.module._anyResolveConflictByUI(path, conflict); + conflictedRevisions.shift(); + + const restartedSession = createModule(conflictedRevisions); + restartedSession.module.onBindFunction(restartedSession.core as never, restartedSession.services as never); + await expect(restartedSession.handlers.unresolvedMessages?.()).resolves.toEqual([ + "This file has unresolved conflicts.", + ]); + + await restartedSession.module._anyResolveConflictByUI(path, { + left: { rev: "3-merged", data: "merged", ctime: 1, mtime: 3 }, + right: { rev: "2-third", data: "third", ctime: 1, mtime: 2 }, + diff: [], + }); + + expect(modalState.constructed).toBe(2); + }); + + it("deletes the compared right leaf when concatenating a deterministically selected pair", async () => { + const { core, module, services } = createModule(["2-unrelated", "2-right"]); + modalState.result = LEAVE_TO_SUBSEQUENT; + core.localDatabase.getDBEntry.mockResolvedValue({ + _rev: "2-left", + _conflicts: ["2-unrelated", "2-right"], + }); + + await module._anyResolveConflictByUI(path, conflict); + + expect(core.databaseFileAccess.storeContent).toHaveBeenCalledWith(path, ""); + expect(services.conflict.resolveByDeletingRevision).toHaveBeenCalledWith(path, "2-right", "UI Concatenated"); + }); + + it("rechecks the live leaves instead of applying a stale dialogue selection", async () => { + const { core, module, services } = createModule(["2-other"]); + modalState.result = "2-right"; + core.localDatabase.getDBEntry.mockResolvedValue({ + _rev: "3-new-winner", + _conflicts: ["2-other"], + }); + + await module._anyResolveConflictByUI(path, conflict); + + expect(services.conflict.resolveByDeletingRevision).not.toHaveBeenCalled(); + expect(services.conflict.queueCheckFor).toHaveBeenCalledWith(path); + }); +}); + +describe("ModuleInteractiveConflictResolver file selection", () => { + beforeEach(() => { + modalState.constructed = 0; + modalState.result = modalState.postponed; + }); + + it("does not show a no-conflicts notice when an automatic repeat reaches its normal end", async () => { + const { core, module } = createModule(); + core.localDatabase.findAllDocs + .mockImplementationOnce(() => + documents([ + { + _id: "note-id", + _rev: "2-left", + _conflicts: ["2-right"], + path, + mtime: 2, + }, + ]) + ) + .mockImplementationOnce(() => documents([])); + core.confirm.askSelectString.mockResolvedValue(path); + + await module.allConflictCheck(); + + expect(core.confirm.askSelectString).toHaveBeenCalledOnce(); + expect(module._log).not.toHaveBeenCalledWith("There are no conflicted documents", LOG_LEVEL_NOTICE); + }); + + it("shows one no-conflicts notice for an explicit selection request which starts empty", async () => { + const { module } = createModule(); + + await module.pickFileForResolve(); + + expect(module._log).toHaveBeenCalledTimes(1); + expect(module._log).toHaveBeenCalledWith("There are no conflicted documents", LOG_LEVEL_NOTICE); + }); }); diff --git a/styles.css b/styles.css index dfc311dc..3f315599 100644 --- a/styles.css +++ b/styles.css @@ -17,8 +17,16 @@ min-width: 5em; } +.conflict-action-container { + display: flex; + flex-direction: column; + gap: var(--size-4-2); + margin-top: var(--size-4-2); +} + .conflict-action-button { - margin-right: 4px; + width: 100%; + margin: 0; } .op-scrollable { diff --git a/test/e2e-obsidian/README.md b/test/e2e-obsidian/README.md index b0acbff1..3ea9745c 100644 --- a/test/e2e-obsidian/README.md +++ b/test/e2e-obsidian/README.md @@ -132,7 +132,7 @@ LIVESYNC_CLI_COMMAND="docker run --rm --network host --user $(id -u):$(id -g) -- `test:e2e:obsidian:two-vault-sync` runs a two-vault note synchronisation workflow. It verifies note creation, update, ordinary rename, a case-only file name change within the same directory, deletion, and a separate encrypted round-trip with Path Obfuscation enabled. Its target-filter scenario confirms that one Vault receives and checkpoints a remote document without reflecting it, restarts with the same profile and filter, and then reflects the stored document after the filter is broadened through the settings service. Directory case changes deliberately remain outside this scenario because they require directory-aware rename handling. The optional Markdown conflict check can be enabled with `E2E_OBSIDIAN_INCLUDE_MARKDOWN_CONFLICT=true`. It creates divergent revisions in two separate Vaults, performs a conservative merge on one Vault, edits that result again, and requires the other Vault to replace its known deleted losing revision without recreating the conflict. The separate `E2E_OBSIDIAN_INCLUDE_CONFLICT_OPERATIONS=true` check keeps four conflicts active while one Vault edits, deletes, performs a case-only rename, and performs a cross-path rename. It asserts that each operation extends the revision displayed on that device, replicates the exact resulting revision tree, and preserves the other live branch. During focused development, `E2E_OBSIDIAN_ONLY_CONFLICT_OPERATIONS=true` runs that self-contained scope without the ordinary, target-filter, or encrypted scenarios. Both conflict checks remain outside the default local suite. -`test:e2e:obsidian:conflict-dialog-policy` creates real local revision conflicts without a remote service, opens the merge dialogue in Obsidian, and chooses **Not now**. It verifies that an ordinary repeated conflict check does not reopen the dialogue, that the active editor retains the unresolved-conflict warning, and that **Resolve if conflicted.** explicitly reopens the dialogue. It then invokes the same Commonlib consumer boundary used for an incoming replicated document and verifies both resolution-arrival cases: a postponed warning disappears, and an open stale dialogue closes, once the local revision tree has no conflict leaf. This isolates the Obsidian UI contract from transport and second-device setup. The fixture owns one temporary Vault and profile, and the session runner stops Obsidian before removing them. +`test:e2e:obsidian:conflict-dialog-policy` creates three real local revision leaves without a remote service and opens the pairwise merge dialogue in Obsidian. It verifies the three-version count, requires the four decision buttons to be stacked vertically, concatenates the displayed pair as a child of the displayed winner, confirms that the untouched leaf remains as one conflict, postpones that remaining pair, restarts the same isolated Vault and profile, and confirms that only the two live versions are reconstructed. It also verifies that an ordinary repeated conflict check does not reopen a postponed dialogue, that **Resolve if conflicted.** explicitly reopens it, and that the active editor retains the appropriate unresolved-conflict warning. The scenario then invokes the same Commonlib consumer boundary used for an incoming replicated document and checks that a postponed warning disappears, an open stale dialogue closes, and the conflict-processing queue completes even when the dialogue closes immediately. This isolates the Obsidian UI contract from transport and second-device setup. The fixture owns one temporary Vault and profile, and the session runner stops Obsidian before removing them. `test:e2e:obsidian:hidden-file-snippet-sync` runs a two-vault hidden file round-trip. It verifies creation and deletion of a real `.obsidian/snippets/*.css` file, automatic JSON conflict merging for a hidden file with the merged result propagated by a second synchronisation, manual JSON Resolve dialogue application through Obsidian's UI, and per-device target patterns where one vault ignores a hidden file that the other vault synchronises. It also covers [issue #555](https://github.com/vrtmrz/obsidian-livesync/issues/555) by requiring several plug-in and settings changes to share one mobile-safe Notice with actionable touch targets; a manually dismissed group must not repeat its acknowledged rows when a later change arrives. diff --git a/test/e2e-obsidian/scripts/conflict-dialog-policy.ts b/test/e2e-obsidian/scripts/conflict-dialog-policy.ts index 115f6f02..254256bc 100644 --- a/test/e2e-obsidian/scripts/conflict-dialog-policy.ts +++ b/test/e2e-obsidian/scripts/conflict-dialog-policy.ts @@ -13,10 +13,12 @@ const path = "conflict-dialog-policy.md"; const baseContent = "Conflict dialogue policy\n\nShared base.\n"; const leftContent = "Conflict dialogue policy\n\nChanged on the left.\n"; const rightContent = "Conflict dialogue policy\n\nChanged on the right.\n"; +const thirdContent = "Conflict dialogue policy\n\nChanged on the third branch.\n"; const uiTimeoutMs = Number(process.env.E2E_OBSIDIAN_CONFLICT_DIALOG_TIMEOUT_MS ?? 10000); type ConflictFixture = { currentRev: string; + currentParentRev?: string; conflicts: string[]; }; @@ -46,7 +48,8 @@ async function createAndOpenBaseFile(cliBinary: string, env: NodeJS.ProcessEnv): async function createManualConflict( cliBinary: string, env: NodeJS.ProcessEnv, - baseRev: string + baseRev: string, + contents: readonly string[] ): Promise { return await evalObsidianJson( cliBinary, @@ -54,12 +57,12 @@ async function createManualConflict( "(async()=>{", `const path=${JSON.stringify(path)};`, `const baseRev=${JSON.stringify(baseRev)};`, - `const contents=${JSON.stringify([leftContent, rightContent])};`, + `const contents=${JSON.stringify(contents)};`, "const core=app.plugins.plugins['obsidian-livesync'].core;", "const id=await core.services.path.path2id(path);", - "for(const content of contents){", + "for(const [index,content] of contents.entries()){", " const blob=new Blob([content],{type:'text/plain'});", - " const now=Date.now();", + " const now=Date.now()+index;", " const result=await core.localDatabase.putDBEntry({", " _id:id,path,data:blob,ctime:now,mtime:now,", " size:(await blob.arrayBuffer()).byteLength,children:[],", @@ -69,7 +72,7 @@ async function createManualConflict( "}", "const meta=await core.localDatabase.getDBEntryMeta(path,{conflicts:true},true);", "if(!meta?._rev||!meta._conflicts?.length){", - " throw new Error(`Conflict fixture did not produce two live leaves: ${path}`);", + " throw new Error(`Conflict fixture did not produce multiple live leaves: ${path}`);", "}", "return JSON.stringify({currentRev:meta._rev,conflicts:meta._conflicts});", "})()", @@ -78,7 +81,48 @@ async function createManualConflict( ); } -async function requestConflictCheck(cliBinary: string, env: NodeJS.ProcessEnv, waitForCompletion: boolean) { +async function readConflictFixture(cliBinary: string, env: NodeJS.ProcessEnv): Promise { + return await evalObsidianJson( + cliBinary, + [ + "(async()=>{", + `const path=${JSON.stringify(path)};`, + "const core=app.plugins.plugins['obsidian-livesync'].core;", + "const meta=await core.localDatabase.getDBEntryMeta(path,{conflicts:true,revs:true},true);", + "if(!meta?._rev){", + " throw new Error(`Could not read the conflict fixture: ${path}`);", + "}", + "const revisions=meta._revisions;", + "const currentParentRev=revisions?.ids?.length>1", + " ? `${revisions.start-1}-${revisions.ids[1]}`", + " : undefined;", + "return JSON.stringify({currentRev:meta._rev,currentParentRev,conflicts:meta._conflicts??[]});", + "})()", + ].join(""), + env + ); +} + +async function waitForConflictCount( + cliBinary: string, + env: NodeJS.ProcessEnv, + expectedConflictCount: number +): Promise { + const deadline = Date.now() + uiTimeoutMs; + let fixture = await readConflictFixture(cliBinary, env); + while (fixture.conflicts.length !== expectedConflictCount && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 100)); + fixture = await readConflictFixture(cliBinary, env); + } + if (fixture.conflicts.length !== expectedConflictCount) { + throw new Error( + `Expected ${expectedConflictCount + 1} live version(s), but found ${fixture.conflicts.length + 1}: ${JSON.stringify(fixture)}` + ); + } + return fixture; +} + +async function requestConflictCheck(cliBinary: string, env: NodeJS.ProcessEnv, waitForCompletion = false) { await evalObsidianJson( cliBinary, [ @@ -86,9 +130,8 @@ async function requestConflictCheck(cliBinary: string, env: NodeJS.ProcessEnv, w `const path=${JSON.stringify(path)};`, `const waitForCompletion=${JSON.stringify(waitForCompletion)};`, "const core=app.plugins.plugins['obsidian-livesync'].core;", - "const queued=core.services.conflict.queueCheckFor(path);", + "await core.services.conflict.queueCheckFor(path);", "if(waitForCompletion){", - " await queued;", " await core.services.conflict.ensureAllProcessed();", "}", "return JSON.stringify({ok:true});", @@ -98,10 +141,25 @@ async function requestConflictCheck(cliBinary: string, env: NodeJS.ProcessEnv, w ); } +async function waitForConflictChecks(cliBinary: string, env: NodeJS.ProcessEnv): Promise { + await evalObsidianJson( + cliBinary, + [ + "(async()=>{", + "const core=app.plugins.plugins['obsidian-livesync'].core;", + "await core.services.conflict.ensureAllProcessed();", + "return JSON.stringify({ok:true});", + "})()", + ].join(""), + env + ); +} + async function applyReplicatedConflictResolution( cliBinary: string, env: NodeJS.ProcessEnv, - revisionToDelete: string + revisionToDelete: string, + expectedConflictCount = 0 ): Promise { await evalObsidianJson( cliBinary, @@ -109,6 +167,7 @@ async function applyReplicatedConflictResolution( "(async()=>{", `const path=${JSON.stringify(path)};`, `const revisionToDelete=${JSON.stringify(revisionToDelete)};`, + `const expectedConflictCount=${JSON.stringify(expectedConflictCount)};`, "const core=app.plugins.plugins['obsidian-livesync'].core;", "if(!(await core.fileHandler.deleteRevisionFromDB(path,revisionToDelete))){", " throw new Error(`Could not apply the replicated conflict resolution: ${path} ${revisionToDelete}`);", @@ -122,8 +181,8 @@ async function applyReplicatedConflictResolution( // isolates the dialogue policy from transport and second-device setup. "await core.fileHandler._anyProcessReplicatedDoc(entry);", "const conflicts=await core.databaseFileAccess.getConflictedRevs(path);", - "if(conflicts.length!==0){", - " throw new Error(`Replicated resolution left conflicts behind: ${path} ${JSON.stringify(conflicts)}`);", + "if(conflicts.length!==expectedConflictCount){", + " throw new Error(`Replicated resolution left an unexpected conflict count: ${path} ${JSON.stringify(conflicts)}`);", "}", "return JSON.stringify({ok:true});", "})()", @@ -179,40 +238,79 @@ async function main(): Promise { await waitForLiveSyncCoreReady(cliBinary, session.cliEnv); await createAndOpenBaseFile(cliBinary, session.cliEnv); const base = await waitForLocalDatabaseEntry(cliBinary, session.cliEnv, path); - const fixture = await createManualConflict(cliBinary, session.cliEnv, base.rev); - if (fixture.conflicts.length !== 1) { - throw new Error(`Expected exactly two live leaves: ${JSON.stringify(fixture)}`); + const fixture = await createManualConflict(cliBinary, session.cliEnv, base.rev, [ + leftContent, + rightContent, + thirdContent, + ]); + if (fixture.conflicts.length !== 2) { + throw new Error(`Expected exactly three live leaves: ${JSON.stringify(fixture)}`); } - await requestConflictCheck(cliBinary, session.cliEnv, false); + await requestConflictCheck(cliBinary, session.cliEnv); await withObsidianPage(session.remoteDebuggingPort, async (page) => { const modal = conflictDialogue(page); await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); - await modal.getByRole("button", { name: "Not now", exact: true }).waitFor({ + await page + .locator(".livesync-status-messagearea") + .filter({ + hasText: "This file has 3 unresolved versions. They will be reviewed one pair at a time.", + }) + .waitFor({ state: "visible", timeout: uiTimeoutMs }); + await modal.getByRole("button", { name: "Concat both", exact: true }).waitFor({ state: "visible", timeout: uiTimeoutMs, }); + const actionButtonBounds = await modal.locator(".conflict-action-button").evaluateAll((buttons) => + buttons.map((button) => { + const bounds = button.getBoundingClientRect(); + return { top: bounds.top, bottom: bounds.bottom }; + }) + ); + if ( + actionButtonBounds.length !== 4 || + actionButtonBounds.some( + (bounds, index) => index > 0 && bounds.top < actionButtonBounds[index - 1].bottom + ) + ) { + throw new Error( + `Conflict action buttons are not stacked vertically: ${JSON.stringify(actionButtonBounds)}` + ); + } }); const firstDialogueScreenshot = await captureObsidianElement( session.remoteDebuggingPort, - "conflict-dialog-before-not-now.png", + "conflict-dialog-three-versions.png", (page) => conflictDialogue(page).locator(".modal").first() ); await withObsidianPage(session.remoteDebuggingPort, async (page) => { const modal = conflictDialogue(page); - await modal.getByRole("button", { name: "Not now", exact: true }).click({ timeout: uiTimeoutMs }); - await modal.waitFor({ state: "hidden", timeout: uiTimeoutMs }); + await modal.getByRole("button", { name: "Concat both", exact: true }).click({ timeout: uiTimeoutMs }); }); - await requestConflictCheck(cliBinary, session.cliEnv, true); + const remainingAfterConcatenation = await waitForConflictCount(cliBinary, session.cliEnv, 1); + if ( + remainingAfterConcatenation.currentRev === fixture.currentRev || + remainingAfterConcatenation.currentParentRev !== fixture.currentRev + ) { + throw new Error( + `Concatenation did not extend the compared winner before retaining the remaining branch: ${JSON.stringify( + { + before: fixture, + after: remainingAfterConcatenation, + } + )}` + ); + } await withObsidianPage(session.remoteDebuggingPort, async (page) => { + const modal = conflictDialogue(page); + await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); const warning = page.locator(".livesync-status-messagearea").filter({ hasText: "This file has unresolved conflicts.", }); await warning.waitFor({ state: "visible", timeout: uiTimeoutMs }); - if (await conflictDialogue(page).isVisible()) { - throw new Error("The postponed conflict dialogue reopened during an ordinary conflict check."); - } + await modal.getByRole("button", { name: "Not now", exact: true }).click({ timeout: uiTimeoutMs }); + await modal.waitFor({ state: "hidden", timeout: uiTimeoutMs }); }); const warningScreenshot = await captureObsidianElement( session.remoteDebuggingPort, @@ -223,42 +321,93 @@ async function main(): Promise { }) ); - await applyReplicatedConflictResolution(cliBinary, session.cliEnv, fixture.conflicts[0]); - await withObsidianPage(session.remoteDebuggingPort, async (page) => { - await conflictDialogue(page).waitFor({ state: "hidden", timeout: uiTimeoutMs }); + await session.app.stop(); + session = undefined; + session = await startObsidianLiveSyncSession({ + binary, + cliBinary, + vault, + startupGraceMs: Number(process.env.E2E_OBSIDIAN_STARTUP_GRACE_MS ?? 1000), + }); + await waitForLiveSyncCoreReady(cliBinary, session.cliEnv); + await createAndOpenBaseFile(cliBinary, session.cliEnv); + const remainingAfterRestart = await waitForConflictCount(cliBinary, session.cliEnv, 1); + + await requestConflictCheck(cliBinary, session.cliEnv); + const restartedSession = session; + await withObsidianPage(restartedSession.remoteDebuggingPort, async (page) => { + const modal = conflictDialogue(page); + await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); + await page + .locator(".livesync-status-messagearea") + .filter({ hasText: "This file has unresolved conflicts." }) + .waitFor({ state: "visible", timeout: uiTimeoutMs }); + await applyReplicatedConflictResolution( + cliBinary, + restartedSession.cliEnv, + remainingAfterRestart.conflicts[0] + ); + await modal.waitFor({ state: "hidden", timeout: uiTimeoutMs }); await page .locator(".livesync-status-messagearea") .filter({ hasText: "This file has unresolved conflicts." }) .waitFor({ state: "hidden", timeout: uiTimeoutMs }); }); + // End the replicated-resolution episode before creating another + // conflict at the same path. This prevents a late cancellation event + // from the first episode from closing the later episode's dialogue. + await session.app.stop(); + session = undefined; + session = await startObsidianLiveSyncSession({ + binary, + cliBinary, + vault, + startupGraceMs: Number(process.env.E2E_OBSIDIAN_STARTUP_GRACE_MS ?? 1000), + }); + await waitForLiveSyncCoreReady(cliBinary, session.cliEnv); + await createAndOpenBaseFile(cliBinary, session.cliEnv); + const resolved = await waitForLocalDatabaseEntry(cliBinary, session.cliEnv, path); - const laterFixture = await createManualConflict(cliBinary, session.cliEnv, resolved.rev); + const laterFixture = await createManualConflict(cliBinary, session.cliEnv, resolved.rev, [ + leftContent, + rightContent, + ]); if (laterFixture.conflicts.length !== 1) { throw new Error(`Expected a later conflict with exactly two live leaves: ${JSON.stringify(laterFixture)}`); } - await requestConflictCheck(cliBinary, session.cliEnv, false); + await requestConflictCheck(cliBinary, session.cliEnv); await withObsidianPage(session.remoteDebuggingPort, async (page) => { const modal = conflictDialogue(page); await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); await modal.getByRole("button", { name: "Not now", exact: true }).click({ timeout: uiTimeoutMs }); await modal.waitFor({ state: "hidden", timeout: uiTimeoutMs }); }); + await waitForConflictChecks(cliBinary, session.cliEnv); - const commandExecuted = await withObsidianPage(session.remoteDebuggingPort, async (page) => { + await requestConflictCheck(cliBinary, session.cliEnv, true); + await withObsidianPage(session.remoteDebuggingPort, async (page) => { + await page.waitForTimeout(1500); + if (await conflictDialogue(page).isVisible()) { + throw new Error("The postponed conflict dialogue reopened during an ordinary conflict check."); + } + }); + await waitForConflictCount(cliBinary, session.cliEnv, 1); + + const laterCommandExecuted = await withObsidianPage(session.remoteDebuggingPort, async (page) => { return await page.evaluate( (commandId) => (globalThis as ObsidianTestGlobal).app?.commands?.executeCommandById(commandId) === true, "obsidian-livesync:livesync-checkdoc-conflicted" ); }); - if (!commandExecuted) { + if (!laterCommandExecuted) { throw new Error("The explicit conflict-resolution command was not registered for the active editor."); } - const activeSession = session; - await withObsidianPage(activeSession.remoteDebuggingPort, async (page) => { + const laterActiveSession = session; + await withObsidianPage(laterActiveSession.remoteDebuggingPort, async (page) => { const modal = conflictDialogue(page); await modal.waitFor({ state: "visible", timeout: uiTimeoutMs }); - await applyReplicatedConflictResolution(cliBinary, activeSession.cliEnv, laterFixture.conflicts[0]); + await applyReplicatedConflictResolution(cliBinary, laterActiveSession.cliEnv, laterFixture.conflicts[0]); await modal.waitFor({ state: "hidden", timeout: uiTimeoutMs }); await page .locator(".livesync-status-messagearea") @@ -267,7 +416,7 @@ async function main(): Promise { }); console.log( - "Real Obsidian retained the conflict warning, suppressed an ordinary repeat prompt, reopened the dialogue after the explicit command, and cleared both postponed and open-dialogue states after replicated resolutions." + "Real Obsidian reviewed three versions pairwise, retained the completed stage across restart, suppressed an ordinary repeat prompt after Not now, reopened the dialogue after the explicit command, and cleared both postponed and open-dialogue states after replicated resolutions." ); console.log(`Dialogue screenshot: ${firstDialogueScreenshot}`); console.log(`Postponed warning screenshot: ${warningScreenshot}`); diff --git a/updates.md b/updates.md index 43f49314..20468ad1 100644 --- a/updates.md +++ b/updates.md @@ -18,16 +18,17 @@ Earlier releases remain available in the [0.25 release history](https://github.c ### Improved -- Choosing **Not now** on a merge conflict now postpones repeated dialogues for that conflict while the active file retains an unresolved-conflict warning. The existing conflict commands can reopen it explicitly, and a later conflict prompts again after the current one has been resolved. +- Choosing **Not now** on a merge conflict now postpones repeated dialogues for that conflict while the active file retains an unresolved-conflict warning. Three or more live versions show their current count and are reviewed one deterministic pair at a time; completed pairs remain resolved across restart. The existing conflict commands can reopen a postponed conflict explicitly, and a later conflict prompts again after the current one has been resolved. ### Fixed - Conflict resolutions made on another device no longer recreate the same conflict when the receiving Vault still contains the exact content of the deleted losing revision. Automatic text and structured-data merge now uses the nearest revision actually shared by both branches instead of inferring ancestry from revision generation numbers. - Edits, deletions, and renames made while a file is conflicted now extend the exact revision displayed on that device. If LiveSync cannot prove the displayed branch, it preserves the affected branches for review instead of silently applying the operation to the database winner. +- Answering or externally closing a merge dialogue immediately no longer leaves conflict processing waiting for a response which has already occurred. ### Testing -- Added revision-tree regressions and focused real-Obsidian scenarios for propagated resolutions and file operations performed while a conflict remains active. +- Added revision-tree regressions and focused real-Obsidian scenarios for multiple-version review, restart between resolution stages, propagated resolutions, and file operations performed while a conflict remains active. ## 1.0.0-beta.0