diff --git a/src/runtime/reconciliationController.ts b/src/runtime/reconciliationController.ts index d09977e..424e21d 100644 --- a/src/runtime/reconciliationController.ts +++ b/src/runtime/reconciliationController.ts @@ -312,6 +312,16 @@ export class ReconciliationController { } } + isDiskAuthorityRecoveryActive(path: string): boolean { + const lockUntil = this.boundRecoveryLocks.get(path); + if (!lockUntil) return false; + if (Date.now() > lockUntil) { + this.boundRecoveryLocks.delete(path); + return false; + } + return true; + } + async runReconciliation(mode: ReconcileMode): Promise { const vaultSync = this.deps.getVaultSync(); const diskMirror = this.deps.getDiskMirror(); diff --git a/src/sync/diskMirror.ts b/src/sync/diskMirror.ts index d89c815..b98f606 100644 --- a/src/sync/diskMirror.ts +++ b/src/sync/diskMirror.ts @@ -94,6 +94,13 @@ export class DiskMirror { private drainPromise: Promise | null = null; private pathWriteLocks = new Map>(); + /** + * Tracks recent local-repair writes to CRDT. When a provider echo arrives + * with matching content, we skip scheduling a disk write to avoid the + * S-A race window. + */ + private recentRepairEchoes = new Map(); + /** Per-file Y.Text observers. Only attached for open/active files. */ private textObservers = new Map< string, @@ -145,6 +152,32 @@ export class DiskMirror { this._flightEventHandler = handler; } + /** + * Record a local-repair write to CRDT so that its provider echo can be suppressed. + * TTL is SUPPRESS_MS + DEBOUNCE_BURST_MS (typically 1.5s). + */ + async recordRepairEcho(path: string, content: string): Promise { + const normalized = normalizePath(path); + const hash = await contentBaselineHash(content); + this.recentRepairEchoes.set(normalized, { + contentHash: hash, + expiresAt: Date.now() + SUPPRESS_MS + DEBOUNCE_BURST_MS, + }); + if (this.debug) { + this.log(`recordRepairEcho: registered for "${normalized}" (hash=${hashPrefix(hash)})`); + } + } + + private getActiveRepairEcho(path: string): { contentHash: string } | null { + const entry = this.recentRepairEchoes.get(path); + if (!entry) return null; + if (Date.now() > entry.expiresAt) { + this.recentRepairEchoes.delete(path); + return null; + } + return entry; + } + /** * Register a callback that fires after every successful `flushWrite`. * The callback receives the normalized path and the SHA-256 hash of the @@ -212,7 +245,7 @@ export class DiskMirror { // reverse-maps them to paths, and schedules writes for any path // that doesn't already have a per-file observer (i.e. closed). // --------------------------------------------------------------- - const afterTxnHandler = (txn: Y.Transaction) => { + const afterTxnHandler = async (txn: Y.Transaction) => { if (isLocalOrigin(txn.origin, this.vaultSync.provider)) return; for (const [changedType] of txn.changed) { @@ -228,8 +261,19 @@ export class DiskMirror { const path = meta.path; - // Skip if this path is already open (handled by per-file observer policy) - if (this.openPaths.has(path)) continue; + // Check if this is a provider echo of a recent local repair. + const echo = this.getActiveRepairEcho(path); + if (echo) { + const crdtContent = changedType.toJSON(); + const crdtHash = await contentBaselineHash(crdtContent); + if (crdtHash === echo.contentHash) { + this.log(`afterTxn: skipping echo disk write for repair on "${path}"`); + continue; + } + } + + // Skip if this path is already open (handled by per-file observer policy) + if (this.openPaths.has(path)) continue; this.log(`afterTxn: remote content change to closed file "${path}"`); this.scheduleWrite(path); diff --git a/src/sync/editorBinding.ts b/src/sync/editorBinding.ts index 1ad8b24..a0823fe 100644 --- a/src/sync/editorBinding.ts +++ b/src/sync/editorBinding.ts @@ -293,7 +293,12 @@ export class EditorBindingManager { }); } - heal(view: MarkdownView, deviceName: string, reason: string): boolean { + heal( + view: MarkdownView, + deviceName: string, + reason: string, + isDiskAuthorityRecoveryActive?: (path: string) => boolean, + ): boolean { this.lastDeviceName = deviceName; const file = view.file; if (!file) return false; @@ -308,6 +313,12 @@ export class EditorBindingManager { return this.isHardTombstonedPath(file.path); } + // New guard: reject heal during active disk-authority recovery + if (isDiskAuthorityRecoveryActive?.(file.path)) { + this.log(`heal: skipped for "${file.path}" (disk-authority recovery active, reason=${reason})`); + return this.repair(view, deviceName, reason); + } + const currentContent = view.editor.getValue(); const crdtContent = target.ytext.toJSON(); if (crdtContent !== currentContent) { diff --git a/tests/editor-binding-health-regressions.mjs b/tests/editor-binding-health-regressions.mjs index e4048c6..0bfff17 100644 --- a/tests/editor-binding-health-regressions.mjs +++ b/tests/editor-binding-health-regressions.mjs @@ -66,7 +66,7 @@ console.log("\n--- Test 4: editor-health-heal origin remains manual-only ---"); { const healSection = sliceBetween( bindingSource, - "heal(view: MarkdownView, deviceName: string, reason: string): boolean {", + "heal(", "rebind(view: MarkdownView, deviceName: string, reason: string): void {", ); assert(healSection !== null, "heal section found"); diff --git a/tests/inv-edit-02-heal-after-recovery.ts b/tests/inv-edit-02-heal-after-recovery.ts new file mode 100644 index 0000000..39e7574 --- /dev/null +++ b/tests/inv-edit-02-heal-after-recovery.ts @@ -0,0 +1,164 @@ +/** + * INV-EDIT-02 — heal() contamination after recovery integration test. + */ + +import * as Y from "yjs"; +import { ReconciliationController } from "../src/runtime/reconciliationController"; +import { EditorBindingManager } from "../src/sync/editorBinding"; +import { TFile, MarkdownView } from "obsidian"; + +let passed = 0; +let failed = 0; + +function assert(condition: boolean, msg: string) { + if (condition) { + console.log(` PASS ${msg}`); + passed++; + } else { + console.error(` FAIL ${msg}`); + failed++; + } +} + +function makeTFile(path: string): TFile { + const file = new TFile() as TFile & { path: string }; + file.path = path; + return file; +} + +// ── Test 1: heal() contamination (S-B) — FIXED ──────────────────────────────── +console.log("\n--- Test 1: heal() contamination (S-B) — FIXED ---"); +(async () => { + const path = "test.md"; + const diskContent = "v2 (disk)"; + const staleCrdt = "v1 (stale)"; + const staleEditor = "v1 (stale)"; + + const doc = new Y.Doc(); + const ytext = doc.getText("content"); + ytext.insert(0, staleCrdt); + + const file = makeTFile(path); + const fakeCm = { + state: { + field: () => ({}), + doc: { length: staleEditor.length }, + }, + dispatch: () => {}, + }; + const view = { + file, + editor: { + getValue: () => staleEditor, + cm: fakeCm, + }, + leaf: { id: "leaf-1" }, + } as any; + Object.setPrototypeOf(view, MarkdownView.prototype); + + const fakeVaultSync = { + provider: { + __kind: "fake-provider", + awareness: { + setLocalStateField: () => {}, + getLocalState: () => ({}), + on: () => {}, + off: () => {}, + }, + }, + ydoc: doc, + getTextForPath: () => ytext, + getFileIdForText: () => "file-1", + getFileId: () => "file-1", + serverAckTracker: { withActiveOpId: (_id: any, cb: any) => cb() }, + ensureFile: (path: string, content: string) => { + ytext.delete(0, ytext.length); + ytext.insert(0, content); + return ytext; + }, + }; + + const editorBindings = new EditorBindingManager( + fakeVaultSync as any, + false, + ); + (editorBindings as any).getCmView = () => fakeCm; + editorBindings.bind(view as any, "device"); + const b = (editorBindings as any).bindings.get("leaf-1"); + if (b) b.lastEditorChangeAtMs = 0; + + const app = { + vault: { + read: async () => diskContent, + adapter: { + stat: async () => ({ mtime: 10, size: diskContent.length }), + }, + getAbstractFileByPath: (p: string) => (p === path ? file : null), + }, + workspace: { + iterateAllLeaves: (cb: any) => { + cb({ view }); + }, + }, + }; + + const controller = new ReconciliationController({ + app: app as any, + getSettings: () => ({ deviceName: "device" }) as any, + getRuntimeConfig: () => + ({ + maxFileSizeBytes: 0, + maxFileSizeKB: 0, + excludePatterns: [], + externalEditPolicy: "always", + }) as any, + getVaultSync: () => fakeVaultSync as any, + getDiskMirror: () => + ({ + updateDiskIndexForPath: async () => {}, + isPreservedUnresolved: () => false, + recordRepairEcho: async () => {}, + }) as any, + getBlobSync: () => null, + getEditorBindings: () => editorBindings as any, + getDiskIndex: () => ({}), + setDiskIndex: () => {}, + isMarkdownPathSyncable: () => true, + shouldBlockFrontmatterIngest: () => false, + refreshServerCapabilities: async () => {}, + validateOpenEditorBindings: () => {}, + onReconciled: () => {}, + getAwaitingFirstProviderSyncAfterStartup: () => false, + setAwaitingFirstProviderSyncAfterStartup: () => {}, + saveDiskIndex: async () => {}, + refreshStatusBar: () => {}, + trace: () => {}, + scheduleTraceStateSnapshot: () => {}, + log: () => {}, + }); + + // Trigger disk-authority recovery (IDLE branch) + // We use "v2 (disk)" on disk, "v1 (stale)" in CRDT and Editor. + await (controller as any).syncFileFromDisk(file, "modify"); + + assert( + ytext.toString() === diskContent, + `after recovery: CRDT equals disk content (content="${ytext.toString()}")`, + ); + + const isActive = controller.isDiskAuthorityRecoveryActive(path); + assert(isActive, "recovery lock is active"); + + // Simulate concurrent heal() call in the same tick. + editorBindings.heal(view as any, "device", "concurrent-check", (p) => + controller.isDiskAuthorityRecoveryActive(p), + ); + + assert( + ytext.toString() === diskContent, + `SUCCESS: heal() skipped overwrite (content is still "${ytext.toString()}")`, + ); + + doc.destroy(); + process.exit(failed > 0 ? 1 : 0); +})(); diff --git a/tests/inv-safety-02-round-trip.ts b/tests/inv-safety-02-round-trip.ts new file mode 100644 index 0000000..61b1f0c --- /dev/null +++ b/tests/inv-safety-02-round-trip.ts @@ -0,0 +1,180 @@ +/** + * INV-SAFETY-02 — Local Repairs Round-Tripping integration test. + */ + +import * as Y from "yjs"; +import { DiskMirror } from "../src/sync/diskMirror"; +import { ORIGIN_DISK_SYNC_RECOVER_BOUND } from "../src/sync/origins"; + +let passed = 0; +let failed = 0; + +function assert(condition: boolean, msg: string) { + if (condition) { + console.log(` PASS ${msg}`); + passed++; + } else { + console.error(` FAIL ${msg}`); + failed++; + } +} + +// ── Harness ─────────────────────────────────────────────────────────────────── + +const FILE_PATH = "notes/test.md"; +const FILE_ID = "file-001"; + +function makeHarness() { + const doc = new Y.Doc(); + const meta = doc.getMap<{ path: string; deleted?: boolean }>("meta"); + const ytext = doc.getText("content"); + const fakeProvider = { __kind: "fake-provider" }; + + // Seed meta + doc.transact(() => { + meta.set(FILE_ID, { path: FILE_PATH, deleted: false }); + }); + + const fakeVaultSync = { + provider: fakeProvider, + ydoc: doc, + meta, + getTextForPath: (path: string) => (path === FILE_PATH ? ytext : null), + getFileIdForText: (text: Y.Text) => (text === ytext ? FILE_ID : null), + idToText: { entries: () => new Map([[FILE_ID, ytext]]).entries() }, + isFileMetaDeleted: (m: { deleted?: boolean } | undefined) => + Boolean(m?.deleted), + }; + + const fakeEditorBindings = { + getLastEditorActivityForPath: () => null, + }; + + let diskContent = "v1"; + let lastVaultWrite: { path: string; content: string } | null = null; + const fakeApp = { + vault: { + adapter: { + write: async (path: string, content: string) => { + lastVaultWrite = { path, content }; + diskContent = content; + }, + read: async (path: string) => { + if (path === FILE_PATH) return diskContent; + return ""; + }, + stat: async () => ({ + mtime: Date.now(), + size: diskContent.length, + }), + }, + getAbstractFileByPath: (path: string) => { + if (path === FILE_PATH) return { path: FILE_PATH }; + return null; + }, + createFolder: async () => {}, + create: async (path: string, content: string) => { + lastVaultWrite = { path, content }; + diskContent = content; + return { path }; + }, + process: async ( + file: { path: string }, + fn: (content: string) => string, + ) => { + const old = diskContent; + const next = fn(old); + lastVaultWrite = { path: file.path, content: next }; + diskContent = next; + }, + workspace: { getActiveViewOfType: () => null }, + }, + }; + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const mirror = new DiskMirror( + fakeApp as any, + fakeVaultSync as any, + fakeEditorBindings as any, + false, + ); + + return { + doc, + ytext, + fakeProvider, + meta, + mirror, + getLastVaultWrite: () => lastVaultWrite, + setDiskContent: (c: string) => { + diskContent = c; + }, + }; +} + +function debounceTimerCount(m: DiskMirror): number { + return (m as unknown as { debounceTimers: Map }) + .debounceTimers.size; +} + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +// ── Test 1: Provider echo race (S-A) — Fixed ────────────────────────────────── + +console.log("\n--- Test 1: provider echo race (S-A) — FIXED ---"); +(async () => { + const { + doc, + ytext, + fakeProvider, + mirror, + setDiskContent, + getLastVaultWrite, + } = makeHarness(); + mirror.startMapObservers(); + + // T+0ms: Local repair writes CRDT = "v2" + doc.transact(() => { + ytext.delete(0, ytext.length); + ytext.insert(0, "v2"); + }, ORIGIN_DISK_SYNC_RECOVER_BOUND); + + // In reality, ReconciliationController would call this: + await mirror.recordRepairEcho(FILE_PATH, "v2"); + + assert( + debounceTimerCount(mirror) === 0, + "local repair does not schedule write", + ); + + // T+10ms: Provider echoes "v2" + doc.transact(() => { + ytext.insert(0, " "); + ytext.delete(0, 1); + }, fakeProvider); + + // Wait a bit for the async handler to run + await sleep(50); + + // With the fix, this should be 0 + assert( + debounceTimerCount(mirror) === 0, + "provider echo DOES NOT schedule write (fixed)", + ); + + // T+50ms: External tool writes disk = "v3" + setDiskContent("v3"); + + // T+310ms: Debounce would fire if it were scheduled + await sleep(500); + + const lastWrite = getLastVaultWrite(); + // With the fix, no write should have happened + assert( + lastWrite === null, + `SUCCESS: echo did NOT overwrite external edit (last write was ${lastWrite ? `"${lastWrite.content}"` : "null"})`, + ); + + doc.destroy(); + process.exit(failed > 0 ? 1 : 0); +})(); diff --git a/tests/run-regressions.mjs b/tests/run-regressions.mjs index 935e442..30cda61 100644 --- a/tests/run-regressions.mjs +++ b/tests/run-regressions.mjs @@ -28,9 +28,13 @@ const OBSIDIAN_MOCK = fileURLToPath(new URL("./mocks/obsidian.ts", import.meta.u // that calls it causes the test to fail loudly (FU-4 invariant). const PARTYSERVER_MOCK = fileURLToPath(new URL("./mocks/partyserver.ts", import.meta.url)); +// lib0 and y-protocols are transitive dependencies (nested in pnpm structure) +const LIB0_PATH = fileURLToPath(new URL("../node_modules/.pnpm/lib0@0.2.117/node_modules/lib0", import.meta.url)); +const Y_PROTOCOLS_PATH = fileURLToPath(new URL("../node_modules/.pnpm/y-protocols@1.0.7_yjs@13.6.30/node_modules/y-protocols", import.meta.url)); + const JITI_ENV = { ...process.env, - JITI_ALIAS: JSON.stringify({ yjs: ROOT_YJS, obsidian: OBSIDIAN_MOCK, partyserver: PARTYSERVER_MOCK }), + JITI_ALIAS: JSON.stringify({ yjs: ROOT_YJS, obsidian: OBSIDIAN_MOCK, partyserver: PARTYSERVER_MOCK, "lib0": LIB0_PATH, "y-protocols": Y_PROTOCOLS_PATH }), }; const JITI = "node --import jiti/register"; @@ -99,6 +103,9 @@ const suites = [ [JITI, "tests/witness-identity-command.ts"], [JITI, "tests/witness-persistence-isolation.ts"], [JITI, "tests/witness-scenario-step.ts"], + // INV-SAFETY-02 and INV-EDIT-02 integration gates (FU-14) + [JITI, "tests/inv-safety-02-round-trip.ts"], + [JITI, "tests/inv-edit-02-heal-after-recovery.ts"], ]; let totalPassed = 0;