diff --git a/__tests__/BackgroundSync.canvas-session-release.test.ts b/__tests__/BackgroundSync.canvas-session-release.test.ts new file mode 100644 index 00000000..a465386e Binary files /dev/null and b/__tests__/BackgroundSync.canvas-session-release.test.ts differ diff --git a/__tests__/BackgroundSync.download-retry.test.ts b/__tests__/BackgroundSync.download-retry.test.ts new file mode 100644 index 00000000..1a43829f Binary files /dev/null and b/__tests__/BackgroundSync.download-retry.test.ts differ diff --git a/__tests__/CAS.file-transfer-retry.test.ts b/__tests__/CAS.file-transfer-retry.test.ts new file mode 100644 index 00000000..06d5fe63 Binary files /dev/null and b/__tests__/CAS.file-transfer-retry.test.ts differ diff --git a/__tests__/LiveTokenStore.file-token-classification.test.ts b/__tests__/LiveTokenStore.file-token-classification.test.ts new file mode 100644 index 00000000..cfab9c3d Binary files /dev/null and b/__tests__/LiveTokenStore.file-token-classification.test.ts differ diff --git a/__tests__/SharedFolder.remote-delete-teardown.test.ts b/__tests__/SharedFolder.remote-delete-teardown.test.ts new file mode 100644 index 00000000..1deee241 Binary files /dev/null and b/__tests__/SharedFolder.remote-delete-teardown.test.ts differ diff --git a/__tests__/SharedFolder.stale-hold-sweep.test.ts b/__tests__/SharedFolder.stale-hold-sweep.test.ts new file mode 100644 index 00000000..8554410e Binary files /dev/null and b/__tests__/SharedFolder.stale-hold-sweep.test.ts differ diff --git a/__tests__/SharedFolder.test.ts b/__tests__/SharedFolder.test.ts index a36d35b5..87395c88 100644 Binary files a/__tests__/SharedFolder.test.ts and b/__tests__/SharedFolder.test.ts differ diff --git a/__tests__/SyncFile.test.ts b/__tests__/SyncFile.test.ts index 9ac6f6ed..6629503b 100644 Binary files a/__tests__/SyncFile.test.ts and b/__tests__/SyncFile.test.ts differ diff --git a/__tests__/merge-hsm/MergeHSM.test.ts b/__tests__/merge-hsm/MergeHSM.test.ts index 0e56218b..f9c63f93 100644 Binary files a/__tests__/merge-hsm/MergeHSM.test.ts and b/__tests__/merge-hsm/MergeHSM.test.ts differ diff --git a/__tests__/merge-hsm/frontmatter-mirror-divergence.test.ts b/__tests__/merge-hsm/frontmatter-mirror-divergence.test.ts new file mode 100644 index 00000000..ab367eb3 Binary files /dev/null and b/__tests__/merge-hsm/frontmatter-mirror-divergence.test.ts differ diff --git a/__tests__/merge-hsm/table-cell-sub-editor.test.ts b/__tests__/merge-hsm/table-cell-sub-editor.test.ts new file mode 100644 index 00000000..b7638e90 Binary files /dev/null and b/__tests__/merge-hsm/table-cell-sub-editor.test.ts differ diff --git a/__tests__/merge-hsm/testing/fakeEditorHarness.ts b/__tests__/merge-hsm/testing/fakeEditorHarness.ts index 811b288c..4c922f64 100644 Binary files a/__tests__/merge-hsm/testing/fakeEditorHarness.ts and b/__tests__/merge-hsm/testing/fakeEditorHarness.ts differ diff --git a/__tests__/y-codemirror.next/UserAttributionPlugin.test.ts b/__tests__/y-codemirror.next/UserAttributionPlugin.test.ts new file mode 100644 index 00000000..5cc811d6 Binary files /dev/null and b/__tests__/y-codemirror.next/UserAttributionPlugin.test.ts differ diff --git a/src/BackgroundSync.ts b/src/BackgroundSync.ts index 28845a27..5966ddb4 100644 --- a/src/BackgroundSync.ts +++ b/src/BackgroundSync.ts @@ -63,6 +63,14 @@ export interface BackgroundSyncFailure { kind: "sync" | "download" | "local"; message: string; sharedFolder: SharedFolder; + /** + * Whether the recorded error was a transient class (5xx, throttling, + * network-level). Transient failures stay claimable: the periodic pass + * re-enqueues them once the reclaim interval has elapsed. Permanent + * classes (auth/permission) are never re-driven automatically. + */ + retryable: boolean; + recordedAt: number; } export interface SyncGroup { @@ -127,6 +135,11 @@ const BACKGROUND_SYNC_QUEUE_PUMP_INTERVAL_MS = 1000; const BACKGROUND_SYNC_FOLDER_POLL_INTERVAL_MS = 5000; const BACKGROUND_SYNC_DRAIN_BUDGET_MS = 8; const LOCAL_AHEAD_RETRY_INTERVAL_MS = 5 * 60_000; +// How long a terminally-failed file transfer rests before the periodic pass +// re-enqueues it. Short-lived blips are already absorbed by the queue's own +// backoff retries; this interval is the long-tail self-heal for outages that +// outlast them, so it can be generous without stranding files until reload. +const SYNC_FILE_RECLAIM_INTERVAL_MS = 5 * 60_000; // A provider-bound sync or download operation that has not settled in this long // is treated as timed out. Generous by design — a healthy but slow transfer // must never trip it — because the deadline only detects a wedged await (a dead @@ -271,6 +284,7 @@ export class BackgroundSync extends HasLogging { this.sharedFolders.forEach((folder) => { folder.poll(); }); + this.reclaimStalledSyncFiles(); }, BACKGROUND_SYNC_FOLDER_POLL_INTERVAL_MS); this.subscriptions.push( @@ -633,10 +647,90 @@ export class BackgroundSync extends HasLogging { return true; } + private requeueRetryableDownload( + item: QueueItem, + error: Error, + ): boolean { + const retries = (item.retryAttempts ?? 0) + 1; + item.retryAttempts = retries; + if (retries > MAX_PROVIDER_SYNC_RETRIES) { + item.nextAttemptAt = undefined; + item.retryReason = undefined; + this.warn( + `[download] retryable download failed after ${MAX_PROVIDER_SYNC_RETRIES} retries for ${item.path}: ${error.message}`, + ); + return false; + } + + const delayMs = Math.min(30_000, 1000 * 2 ** Math.min(retries - 1, 5)); + const reason = this.retryReason(error); + item.status = "pending"; + item.nextAttemptAt = this.timeProvider.now() + delayMs; + item.retryReason = reason; + metrics.recordBgSyncRetry("download", reason, retries, delayMs / 1000); + + this.clearFailure(this.failureKey("download", item.guid)); + if (!this.downloadQueue.some((queued) => queued.guid === item.guid)) { + this.downloadQueue.push(item); + this.sortByPath(this.downloadQueue, "download", "retry"); + } + this.debug( + `[download] retryable download failure for ${item.path}: ${error.message}; retrying in ${delayMs}ms`, + ); + metrics.setBgSyncQueueLength("download", this.downloadQueue.length); + this.queueStatusChanged.notifyListeners(); + return true; + } + private retryReason(error: Error): "provider" | "s3" { return isRetryableProviderSyncError(error) ? "provider" : "s3"; } + /** + * A file transfer that exhausted its queue retries must stay claimable: + * nothing else re-enqueues an unchanged file within a session (the folder + * poll covers documents and canvases; membership deltas only fire when + * metadata changes), so without this pass one outage lasting longer than + * the backoff window strands the file until plugin reload. Re-enqueue + * transient failures once the reclaim interval has elapsed. Permanent + * classes stay parked: retrying cannot heal an auth or permission + * refusal, and re-driving them would ping the server forever. + */ + reclaimStalledSyncFiles(): void { + const now = this.timeProvider.now(); + for (const failure of this.failures.values()) { + if (failure.kind === "local" || !failure.retryable) continue; + if (now - failure.recordedAt < SYNC_FILE_RECLAIM_INTERVAL_MS) continue; + const file = failure.sharedFolder.files.get(failure.guid); + if (!isSyncFile(file) || file.destroyed) continue; + if ( + !failure.sharedFolder.connected || + failure.sharedFolder.intent === "disconnected" + ) { + continue; + } + if ( + this.inProgressSyncs.has(failure.guid) || + this.inProgressDownloads.has(failure.guid) + ) { + continue; + } + this.debug( + `[reclaim] re-enqueueing stalled file transfer for ${failure.path}`, + ); + if (failure.kind === "download") { + this.enqueueDownload(file, false).catch(() => { + // The failure is re-recorded by the queue; the next reclaim + // pass paces itself from the fresh record. + }); + } else { + this.enqueueSync(file).catch(() => { + // Same: the queue re-records the failure on rejection. + }); + } + } + } + private recordTickDelay( tick: "queue" | "folder_poll", lastTickAt: number, @@ -840,6 +934,8 @@ export class BackgroundSync extends HasLogging { kind: "local", message, sharedFolder, + retryable: false, + recordedAt: this.timeProvider.now(), }); } else { this.clearFailure(id); @@ -1253,9 +1349,12 @@ export class BackgroundSync extends HasLogging { metrics.setBgSyncQueueLength("download", this.downloadQueue.length); - // Filter for items with connected folders - const connectableItems = this.downloadQueue.filter((item) => - this.isDrainable(item), + // Filter for items with connected folders whose backoff has elapsed + const now = this.timeProvider.now(); + const connectableItems = this.downloadQueue.filter( + (item) => + this.isDrainable(item) && + (item.nextAttemptAt === undefined || item.nextAttemptAt <= now), ); while ( @@ -1271,6 +1370,8 @@ export class BackgroundSync extends HasLogging { ); this.observeItemStart("download", item, this.timeProvider.now()); + item.nextAttemptAt = undefined; + item.retryReason = undefined; itemsStarted++; item.status = "running"; const opStart = performance.now(); @@ -1319,6 +1420,13 @@ export class BackgroundSync extends HasLogging { return; } + if ( + isRetryableSyncError(error) && + this.requeueRetryableDownload(item, error) + ) { + return; + } + item.status = "failed"; metrics.incBgSyncOps("download", "failed"); @@ -1337,8 +1445,12 @@ export class BackgroundSync extends HasLogging { metrics.observeBgSyncOp("download", (performance.now() - opStart) / 1000); this.activeDownloads.delete(item); metrics.setBgSyncActive("download", this.activeDownloads.size); - this.inProgressDownloads.delete(item.guid); - this.cancelledDownloads.delete(item.guid); + // A requeued retry keeps its in-progress entry so callers + // sharing the completion promise stay attached to it. + if (!this.downloadQueue.some((queued) => queued.guid === item.guid)) { + this.inProgressDownloads.delete(item.guid); + this.cancelledDownloads.delete(item.guid); + } // Continue queue draining without relying on throttled timers. queueMicrotask(() => { @@ -1359,6 +1471,16 @@ export class BackgroundSync extends HasLogging { continue; } + if ( + isRetryableSyncError(error) && + this.requeueRetryableDownload(item, error) + ) { + metrics.observeBgSyncOp("download", (performance.now() - opStart) / 1000); + this.activeDownloads.delete(item); + metrics.setBgSyncActive("download", this.activeDownloads.size); + continue; + } + item.status = "failed"; metrics.incBgSyncOps("download", "failed"); metrics.observeBgSyncOp("download", (performance.now() - opStart) / 1000); @@ -2295,7 +2417,7 @@ export class BackgroundSync extends HasLogging { }) : false; const shouldCleanupIdleSession = () => - startedDisconnected && + (startedDisconnected || isCanvas(doc)) && !(doc.userLock || sharedFolder?.mergeManager?.isActive(doc.guid)); const cleanupIdleSession = () => { if (isDocument(doc)) { @@ -2312,8 +2434,7 @@ export class BackgroundSync extends HasLogging { } } if (!shouldCleanupIdleSession()) return; - doc.disconnect(); - sharedFolder?.tokenStore.removeFromRefreshQueue(refreshQueueKey); + doc.releaseIdleSession(); }; if (doc.destroyed) return false; const connected = await doc.connect(); @@ -2386,7 +2507,6 @@ export class BackgroundSync extends HasLogging { await this.maybeBootstrapDocumentLCA(doc, token); } - // promise can take some time if (shouldCleanupIdleSession()) { cleanupIdleSession(); } @@ -3018,6 +3138,8 @@ export class BackgroundSync extends HasLogging { kind, message: this.errorMessage(error), sharedFolder: item.sharedFolder, + retryable: isRetryableSyncError(error), + recordedAt: this.timeProvider.now(), }); } @@ -3029,8 +3151,11 @@ export class BackgroundSync extends HasLogging { existing.path === failure.path && existing.kind === failure.kind && existing.message === failure.message && - existing.sharedFolder === failure.sharedFolder + existing.sharedFolder === failure.sharedFolder && + existing.retryable === failure.retryable ) { + // Keep the original recordedAt: an identical failure re-recorded + // paces its reclaim from the first occurrence, not the latest. return; } this.failures.set(failure.id, failure); diff --git a/src/CAS.ts b/src/CAS.ts index 32bc7aea..973b53e9 100644 --- a/src/CAS.ts +++ b/src/CAS.ts @@ -5,18 +5,38 @@ import type { SyncFile } from "./SyncFile"; import { customFetch } from "./customFetch"; import PocketBase from "pocketbase"; import { HasLogging } from "./debug"; -import { s3ApiErrorFromResponse, s3ApiErrorFromUnknown } from "./S3Error"; +import { + isRetryableS3Error, + s3ApiErrorFromResponse, + s3ApiErrorFromUnknown, + s3NetworkFailureFromUnknown, +} from "./S3Error"; +// In-attempt retry schedule for transient transfer failures: one retry per +// entry, each entry the delay cap for that round. Full jitter spreads the +// retries of a bulk transfer hitting one server blip apart instead of +// re-hammering in lockstep. +const TRANSFER_RETRY_DELAYS_MS: readonly number[] = [500, 2000]; + +export interface ContentAddressedStoreOptions { + transferRetryDelaysMs?: readonly number[]; +} export class ContentAddressedStore extends HasLogging { private pb: PocketBase; private tokenStore: LiveTokenStore; + private transferRetryDelaysMs: readonly number[]; - constructor(private sharedFolder: SharedFolder) { + constructor( + private sharedFolder: SharedFolder, + options?: ContentAddressedStoreOptions, + ) { super(); const authUrl = sharedFolder.loginManager.getEndpointManager().getAuthUrl(); this.pb = new PocketBase(authUrl, sharedFolder.loginManager.authStore); this.tokenStore = sharedFolder.tokenStore; + this.transferRetryDelaysMs = + options?.transferRetryDelaysMs ?? TRANSFER_RETRY_DELAYS_MS; } async verify(syncFile: SyncFile): Promise { @@ -47,32 +67,73 @@ export class ContentAddressedStore extends HasLogging { throw new Error("cannot pull file with missing hash"); } const sha256 = syncFile.meta.hash; - const token = await this.tokenStore.getFileToken( - S3RN.encode(syncFile.s3rn), - sha256, - syncFile.mimetype, - 0, - ); - const response = await customFetch(token.baseUrl + "/download-url", { - method: "GET", - headers: { Authorization: `Bearer ${token.token}` }, - relayNetworkDomain: "relay", - }); - if (!response.ok) { - throw new Error( - `[${this.sharedFolder.path}] File download-url failed: ${response.status} for ${syncFile.guid} ${syncFile.meta.hash} ${syncFile.meta.type}`, + const mimetype = syncFile.mimetype; + const documentId = S3RN.encode(syncFile.s3rn); + const meta = syncFile.meta; + return this.withTransientRetry("download attachment", async () => { + const token = await this.tokenStore.getFileToken( + documentId, + sha256, + mimetype, + 0, ); + const response = await customFetch(token.baseUrl + "/download-url", { + method: "GET", + headers: { Authorization: `Bearer ${token.token}` }, + relayNetworkDomain: "relay", + }); + if (!response.ok) { + this.debug( + `[${this.sharedFolder.path}] File download-url failed: ${response.status} for ${syncFile.guid} ${meta.hash} ${meta.type}`, + ); + throw await this.s3ResponseError(response, "download attachment url"); + } + const responseJson = await response.json(); + const presignedUrl = responseJson.downloadUrl; + const downloadResponse = await this.s3Request( + () => customFetch(presignedUrl, { relayNetworkDomain: "external" }), + "download attachment", + ); + if (!downloadResponse.ok) { + throw await this.s3ResponseError( + downloadResponse, + "download attachment", + ); + } + return downloadResponse.arrayBuffer(); + }); + } + + /** + * Run a transfer, retrying transient failures (5xx/throttle-class answers + * and network-level transport errors) a bounded number of times with + * jittered backoff. Permanent classes (auth/permission) are never + * retried. Whatever finally escapes is classified, so the background + * queue above can decide whether to re-drive the operation later. + */ + private async withTransientRetry( + operation: string, + request: () => Promise, + ): Promise { + for (let attempt = 0; ; attempt++) { + try { + return await request(); + } catch (error) { + const classified = + s3NetworkFailureFromUnknown(error, operation) ?? error; + const delayCapMs = this.transferRetryDelaysMs[attempt]; + if (!isRetryableS3Error(classified) || delayCapMs === undefined) { + throw classified; + } + // Full jitter within the round's cap. + const delayMs = Math.floor(Math.random() * (delayCapMs + 1)); + this.debug( + `transient ${operation} failure (attempt ${attempt + 1}); retrying in ${delayMs}ms`, + classified, + ); + await new Promise((resolve) => setTimeout(resolve, delayMs)); + } } - const responseJson = await response.json(); - const presignedUrl = responseJson.downloadUrl; - const downloadResponse = await this.s3Request( - () => customFetch(presignedUrl, { relayNetworkDomain: "external" }), - "download attachment", - ); - if (!downloadResponse.ok) { - throw await this.s3ResponseError(downloadResponse, "download attachment"); - } - return downloadResponse.arrayBuffer(); } async writeFile(syncFile: SyncFile): Promise { @@ -93,10 +154,10 @@ export class ContentAddressedStore extends HasLogging { headers: { Authorization: `Bearer ${token.token}` }, relayNetworkDomain: "relay", }); - const responseJson = await response.json(); if (response.status !== 200) { - throw new Error(responseJson.error); + throw await this.s3ResponseError(response, "upload attachment url"); } + const responseJson = await response.json(); const presignedUrl = responseJson.uploadUrl; const uploadResponse = await this.s3Request( () => @@ -121,7 +182,11 @@ export class ContentAddressedStore extends HasLogging { try { return await request(); } catch (error) { - throw s3ApiErrorFromUnknown(error, operation) ?? error; + throw ( + s3ApiErrorFromUnknown(error, operation) ?? + s3NetworkFailureFromUnknown(error, operation) ?? + error + ); } } diff --git a/src/Canvas.ts b/src/Canvas.ts index c4cd4662..668fafe9 100644 --- a/src/Canvas.ts +++ b/src/Canvas.ts @@ -338,6 +338,7 @@ export class Canvas await trackPromise(`canvasSync:${this.guid}`, this.onceProviderSynced()); if (this.destroyed || !this._materialized) return; await this.markSynced(); + this.releaseIdleSession(); } })().catch((e) => this.warn("canvas provider sync failed", e)); }) @@ -659,6 +660,11 @@ export class Canvas ); } + protected shouldCompleteDeferredDisconnect(): boolean { + if (this.destroyed) return true; + return !this.userLock; + } + public get ready(): boolean { return this._persistence.isReady(this.synced); } diff --git a/src/HasProvider.ts b/src/HasProvider.ts index 4a6e79f7..82d8028e 100644 --- a/src/HasProvider.ts +++ b/src/HasProvider.ts @@ -552,6 +552,14 @@ export class HasProvider extends HasLogging { return true; } + releaseIdleSession(): void { + if (!this.shouldCompleteDeferredDisconnect()) return; + if (!this.deferDisconnectForPendingMessages()) { + this.disconnect(); + } + this.tokenStore.removeFromRefreshQueue(S3RN.encode(this.s3rn)); + } + disconnect() { this.clearDeferredDisconnect(); this.abortProviderSyncWaiters( diff --git a/src/LiveTokenStore.ts b/src/LiveTokenStore.ts index 879de1b0..3d4ffd4e 100644 --- a/src/LiveTokenStore.ts +++ b/src/LiveTokenStore.ts @@ -15,6 +15,7 @@ import { S3RemoteCanvas, } from "./S3RN"; import { customFetch, getRelayRequestHeaders } from "./customFetch"; +import { s3ApiErrorFromResponse } from "./S3Error"; function getJwtExpiryFromClientToken(clientToken: ClientToken): number { // lol this is so fake @@ -235,9 +236,12 @@ export class LiveTokenStore extends TokenStore { }); if (!response.ok) { - debug(response.status, await response.text()); - const responseJSON = await response.json(); - throw new Error(responseJSON.error); + // Read the body once — it can only be consumed a single time — and + // classify by HTTP status so callers can tell a transient server + // failure (retryable) from a permission-class refusal (not). + const body = await response.text(); + debug(response.status, body); + throw s3ApiErrorFromResponse(response.status, body, "file token"); } const clientToken = (await response.json()) as FileToken; diff --git a/src/S3Error.ts b/src/S3Error.ts index a79f3b4b..7f2ad370 100644 --- a/src/S3Error.ts +++ b/src/S3Error.ts @@ -9,6 +9,7 @@ export interface S3ErrorDetails { const RETRYABLE_S3_CODES = new Set([ "InternalError", + "NetworkingError", "RequestTimeout", "ServiceUnavailable", "SlowDown", @@ -57,10 +58,42 @@ export function s3ApiErrorFromResponse( ...parsed, status, operation, - message: parsed?.message, + message: parsed?.message ?? jsonErrorMessage(body), }); } +/** + * Wrap a network-level transport failure (the request never produced an HTTP + * response: DNS, connection reset, dropped socket) as a retryable error. + * Returns null for anything that is not recognizably network-level, so a + * programming error is never classified as transient. Follows the AWS SDK + * convention of classifying such failures as a retryable "NetworkingError". + */ +export function s3NetworkFailureFromUnknown( + error: unknown, + operation?: string, +): S3ApiError | null { + if (error instanceof S3ApiError) return null; + const text = errorText(error); + if (!text || !NETWORK_FAILURE_PATTERN.test(text)) return null; + return new S3ApiError( + { code: "NetworkingError", operation, message: text }, + error, + ); +} + +const NETWORK_FAILURE_PATTERN = + /net::ERR_|Failed to fetch|fetch failed|\bLoad failed\b|NetworkError|network error|socket hang up|ECONNRESET|ECONNREFUSED|ECONNABORTED|ETIMEDOUT|ENOTFOUND|EPIPE|EAI_AGAIN/i; + +function jsonErrorMessage(body: string): string | undefined { + try { + const parsed = JSON.parse(body) as { error?: unknown }; + return typeof parsed?.error === "string" ? parsed.error : undefined; + } catch { + return undefined; + } +} + export function s3ApiErrorFromUnknown( error: unknown, operation?: string, @@ -114,6 +147,8 @@ function userMessageForS3Error(details: S3ErrorDetails): string { return "Attachment storage is busy. Relay will retry the upload."; case "RequestTimeout": return "Attachment storage timed out. Relay will retry the upload."; + case "NetworkingError": + return "Could not reach attachment storage. Relay will retry."; case "AccessDenied": return "Relay could not access attachment storage."; case "ExpiredToken": diff --git a/src/SharedFolder.ts b/src/SharedFolder.ts index ca3cc7d0..cb54f64d 100644 --- a/src/SharedFolder.ts +++ b/src/SharedFolder.ts @@ -321,6 +321,10 @@ export class SharedFolder extends HasProvider { private _firstSyncConverged = false; private _firstSyncConvergedPromise: Promise | undefined; private _resolveFirstSyncConverged: (() => void) | undefined; + /** One-shot flag for the first-sync offline-delete scan. */ + private _offlineDeleteScanDone = false; + /** Paths the scan approved for deletion propagation; valid for one tree sync. */ + private _approvedOfflineDeletes = new Set(); /** Paths removed by provider-applied membership updates before convergence. */ private _preConvergenceRemoteDeletes: Set | undefined; /** Deleted paths that had a local publication hold when convergence opened. */ @@ -2400,7 +2404,19 @@ export class SharedFolder extends HasProvider { // XXX file meta typing if (file && isSyncFile(file) && file.shouldPull(meta as FileMeta)) { - return { op: "update", path, promise: file.pull() }; + // Route through the download queue: it retries transient + // failures with backoff and records terminal ones for the + // periodic reclaim pass, so a failed pull stays claimable + // instead of spending its single attempt here. + const promise = this.backgroundSync + .enqueueDownload(file, false) + .then( + () => undefined, + (error) => { + this.warn(`pull failed for ${path}`, error); + }, + ); + return { op: "update", path, promise }; } // GUID mismatch — file at this path is mapped under a different @@ -2453,6 +2469,23 @@ export class SharedFolder extends HasProvider { } } + // The offline-delete scan approved this path: the file was materialized + // here and removed from disk while the plugin was not running, so + // re-creating it would silently reverse a deletion. Propagate the + // delete instead. + if (this._approvedOfflineDeletes.has(path)) { + this._approvedOfflineDeletes.delete(path); + diffLog.push(`propagating offline deletion of ${path}`); + const promise = this.vault.adapter + .exists(normalizePath(join(this.path, path))) + .then((exists) => { + if (exists) return; + + this.deleteFiles([path]); + }); + return { op: "delete", path, promise }; + } + // write will trigger `create` which will read the file from disk by default. // so we need to pre-empt that by loading the file into docs. const promise = this._handleServerCreate(path, meta, diffLog); @@ -2533,9 +2566,31 @@ export class SharedFolder extends HasProvider { if (synced) { diffLog.push(`deleted local file ${vpath} for remotely deleted doc`); this.markPendingDelete(vpath); - const promise = this.vault.adapter.trashLocal(file.path).finally(() => { - this.clearPendingDelete(vpath); - }); + const promise = this.vault.adapter + .trashLocal(file.path) + .then(() => { + // The pending-delete mark suppresses the trash's own + // vault-delete echo, so the deletion handler that + // would destroy the live in-memory doc never runs for + // this path. A surviving doc re-creates the file on + // its next engine write and re-registers it as new. + // Tear it down here the way a processed vault delete + // would, before the mark clears — the write guard + // covers the window, and a destroyed doc's queued + // writes stand down. + const doc = this.fset.find((f) => f.path === vpath); + if (doc) { + this.fset.delete(doc); + this.files.delete(doc.guid); + doc.cleanup(); + doc.destroy(); + this.teardownDocState(doc.guid); + this.fset.update(); + } + }) + .finally(() => { + this.clearPendingDelete(vpath); + }); deletes.push({ op: "delete", path: vpath, @@ -2997,6 +3052,94 @@ export class SharedFolder extends HasProvider { this.fset.update(); } + /** + * A pending-upload hold whose path already has committed metadata is + * finished business: a matching guid means the publication completed and + * the clear was missed; a different guid means the claim lost its race + * and adoption has had its chance by the end of a converged sync. A + * leaked hold is not inert — it shields the local file from + * remote-delete cleanup and re-publishes the path on the first tree + * sync after its committed meta is deleted (deleted files silently + * reappear) — and its backing storage preserves it across sessions + * indefinitely. + */ + private sweepStalePendingUploads(): void { + if (!(this._provider?.synced && this._persistence?.synced)) return; + + const stale: { + vpath: string; + pending: string; + committed: string; + enrolledUnderPending: boolean; + }[] = []; + this.syncStore.pendingUpload.forEach((guid, vpath) => { + if (this._pendingRemaps.has(vpath)) return; + if (this._convergencePublicationRuns?.has(vpath)) return; + const committed = this.syncStore.getCommittedMeta(vpath); + if (!committed) return; + // A live instance still enrolled under the losing guid means + // adoption stalled; clearing the hold lets path lookups resolve + // to the committed identity and the reconciliation sweep re-key + // it. + stale.push({ + vpath, + pending: guid, + committed: committed.id, + enrolledUnderPending: !!this.files.get(guid), + }); + }); + if (stale.length === 0) return; + + stale.forEach(({ vpath }) => this.pendingUpload.delete(vpath)); + this.warn("dropped stale pending-upload holds", stale); + } + + /** + * Files deleted from disk while the plugin was not running never fire a + * vault delete event, and a committed meta path with no local file is + * otherwise indistinguishable from a download that has not happened yet, + * so the deletion silently reverses ("zombie files"). The persisted HSM + * record is the missing witness: one mapping this guid to the same path + * with disk metadata means this client had the file materialized. + * Runs once, on the first tree sync after the local folder doc loads; + * everything later is covered by live vault events. + */ + private prepareOfflineDeleteScan(): void { + if (this._offlineDeleteScanDone) return; + if (!this._persistence?.synced) return; + const mergeManager = this.mergeManager; + if (!mergeManager) return; + + this._offlineDeleteScanDone = true; + const candidates: string[] = []; + let metaCount = 0; + this.syncStore.forEach((meta, path) => { + metaCount += 1; + if (this.existsSync(path)) return; + const record = mergeManager.getPersistedStateMeta(meta.id); + if (!record?.disk || record.path !== path) return; + if (record.folder && record.folder !== this.guid) return; + candidates.push(path); + }); + if (candidates.length === 0) return; + + // A wholesale disappearance looks less like deletion intent and more + // like a moved or half-restored vault (e.g. restored from an old + // backup while IndexedDB kept the newer records); restore (current + // behavior) rather than propagate a mass delete. The cap is absolute: + // wrongful refusal only means today's resurrection behavior, while + // wrongful propagation deletes fleet-wide. + if (candidates.length > 20) { + this.warn( + `offline-delete scan: ${candidates.length} of ${metaCount} synced files are missing locally; ` + + "refusing to propagate deletions at this scale", + ); + return; + } + candidates.forEach((path) => this._approvedOfflineDeletes.add(path)); + this.log("offline-delete scan: propagating deletions", candidates); + } + syncFileTree(): Promise { // If a sync is already running, mark that we want another sync after if (this.syncFileTreePromise) { @@ -3023,6 +3166,7 @@ export class SharedFolder extends HasProvider { if (!this.mergeManager || this.destroyed) return; await this.mergeManager.initialize(); if (this.destroyed) return; + this.prepareOfflineDeleteScan(); // When file types are newly enabled, enqueue their local // files for syncing before the rest of the tree sync runs. @@ -3084,7 +3228,10 @@ export class SharedFolder extends HasProvider { if (diffLog.length > 0) { this.log("syncFileTree diff:\n" + diffLog.join("\n")); } + this.sweepStalePendingUploads(); } finally { + // Approvals are only valid for the sync pass that computed them. + this._approvedOfflineDeletes.clear(); // Reset the promise after completion (success or failure) this.syncFileTreePromise = null; } @@ -4154,7 +4301,9 @@ export class SharedFolder extends HasProvider { } const file = this.getOrCreateSyncFile(guid, vpath, meta.hash); - this.backgroundSync.enqueueSync(file); + this.backgroundSync.enqueueSync(file).catch((error) => { + this.warn(`sync failed for ${vpath}`, error); + }); this.files.set(guid, file); this.fset.add(file, update); @@ -4179,7 +4328,9 @@ export class SharedFolder extends HasProvider { } const file = this.getOrCreateSyncFile(guid, vpath, meta.hash); - this.backgroundSync.enqueueDownload(file); + this.backgroundSync.enqueueDownload(file, false).catch((error) => { + this.warn(`initial download failed for ${vpath}`, error); + }); this.files.set(guid, file); this.fset.add(file, update); @@ -4260,7 +4411,12 @@ export class SharedFolder extends HasProvider { metaHash: meta.hash, metaSynctime: meta.synctime, }); - file.pull(); + // The queue retries transient failures and records terminal ones + // for the periodic reclaim pass; a bare pull() would spend its one + // attempt and strand the file if the server blipped. + this.backgroundSync.enqueueDownload(file, false).catch((error) => { + this.warn(`initial pull failed for ${vpath}`, error); + }); } this.files.set(guid, file); diff --git a/src/SyncFile.ts b/src/SyncFile.ts index 59637ff3..b7e5dc02 100644 --- a/src/SyncFile.ts +++ b/src/SyncFile.ts @@ -853,9 +853,13 @@ export class SyncFile metaHash: shortHash(this.meta.hash), size: content.byteLength, }); - } catch (e) { - this.log(e); - return; + } catch (error) { + // A failed pull must stay visible and claimable: swallowing the + // error here reported the download as complete, so nothing above + // ever retried and the file stayed missing until plugin reload. + this.uploadError = formatUserFacingError(error, "Failed to pull file"); + this.notifyListeners(); + throw error instanceof Error ? error : errorFromUnknown(error); } } diff --git a/src/SyncStore.ts b/src/SyncStore.ts index 71b3c662..8c16678c 100644 --- a/src/SyncStore.ts +++ b/src/SyncStore.ts @@ -207,6 +207,7 @@ export class SyncStore extends Observable { this.assertVPath(vpath); const guid = uuidv4(); this.pendingUpload.set(vpath, guid); + this.log("minted identity", vpath, guid); return guid; } @@ -329,6 +330,13 @@ export class SyncStore extends Observable { const pendingGuid = this.pendingUpload.get(vpath); if (pendingGuid && pendingGuid === meta.id) { this.pendingUpload.delete(vpath); + } else if (pendingGuid) { + // The pending-upload hold is now stale; if nothing clears it, + // the path re-publishes when this committed entry is deleted. + this.warn("committed claim shadows a pending-upload hold", vpath, { + pending: pendingGuid, + committed: meta.id, + }); } }); } @@ -540,6 +548,11 @@ export class SyncStore extends Observable { if (!meta && this.legacyIds.has(vpath)) { const guid = this.legacyIds.get(vpath)!; + this.warn( + "meta missing but legacy docs entry remains; scheduling meta re-creation", + vpath, + guid, + ); const newMeta = makeDocumentMeta(guid); this.overlay.set(vpath, newMeta); return newMeta; diff --git a/src/flags.ts b/src/flags.ts index 66d9757f..a273906d 100644 --- a/src/flags.ts +++ b/src/flags.ts @@ -16,6 +16,7 @@ export interface FeatureFlags { enableSyncConvergenceLatch: boolean; enableNoteStateInspector: boolean; enableSavingFlagPolyfill: boolean; + enableFrontmatterDuplicateRecovery: boolean; } export type FeatureFlagCategory = "labs" | "debugging" | "danger"; @@ -166,6 +167,13 @@ export const FeatureFlagSchema: { description: "Count overlapping writes per file so Obsidian's saving flag clears when the last write finishes; stands down on versions without the remember/restore bookkeeping.", }, + enableFrontmatterDuplicateRecovery: { + default: false, + category: "danger", + title: "Repair duplicate frontmatter keys", + description: + "Rewrite duplicate top-level YAML keys using last-wins recovery instead of leaving invalid frontmatter for manual repair.", + }, }; export const FeatureFlagDefaults: FeatureFlags = ( diff --git a/src/main.ts b/src/main.ts index 15851cd4..f1f1b8cf 100644 --- a/src/main.ts +++ b/src/main.ts @@ -1048,8 +1048,13 @@ export default class Live extends Plugin { .setTitle("Relay: Download") .setIcon("cloud-download") .onClick(async () => { - await ifile.pull(); - new Notice(`Download complete: ${ifile.name}`); + try { + await ifile.pull(); + new Notice(`Download complete: ${ifile.name}`); + } catch (e) { + this.warn("manual download failed", ifile.path, e); + new Notice(`Download failed: ${ifile.name}`); + } }); }); if (this.debugSettings.get().debugging) { diff --git a/src/merge-hsm/MergeHSM.ts b/src/merge-hsm/MergeHSM.ts index b27cfd17..634d9b0e 100644 --- a/src/merge-hsm/MergeHSM.ts +++ b/src/merge-hsm/MergeHSM.ts @@ -2072,6 +2072,12 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { // Use persistence's initializeWithContent which checks origin in same IDB session const didEnroll = await this.localPersistence!.initializeWithContent!(cachingLoader); + if (didEnroll) { + // This transaction created the document, so its disk text is the + // causal baseline. Returning clients wait for provider sync before + // deriving a missing map from reconciled text. + this.seedFrontmatterMapFromCurrentText(true); + } if (didEnroll && cachedDiskContent) { // Enrollment happened - set LCA to match initial content @@ -2182,6 +2188,18 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { } const hash = await this.hashFn(content); + // Re-asked after the await: a disk change that arrived while the hash + // was computing must not be settled by an enrollment that started + // before it. Completing here would take the ancestor from the server, + // record the enrollment's own hash and mtime as the disk's, and erase + // the only record that the file had already moved on. + if (!this.acceptsRemoteEnrollment) { + this.hsmWarn( + `initial remote enrollment refused after hashing: the document is no longer in a state that accepts one | ` + + `guid=${this._guid} state=${this._statePath}`, + ); + return false; + } this.sendEnrollmentComplete({ contents: content, hash, @@ -3279,6 +3297,7 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { this.pendingEditorContent = null; }, mergeRemoteToLocal: () => this._bridge.flushInbound(), + seedFrontmatterMap: () => this.seedFrontmatterMapFromCurrentText(), repairFrontmatter: () => this.repairFrontmatterFromMap(), absorbTextPreservingRemoteUpdate: (_hsm, event) => this.absorbTextPreservingRemoteUpdate(event as MergeEvent), @@ -3352,6 +3371,17 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { }, storeTwoWayConflict: (_hsm, event) => { const data = (event as any).data; + // The invoke loaded the file to compare against, so its disk + // identity is confirmed even when no disk event preceded the + // conflict. Record it now: a resolution that picks the on-disk + // text produces no later save or disk event, and the capture + // path refuses without a confirmed hash to attach. + if (data.disk) { + this._disk = { hash: data.disk.hash, mtime: data.disk.mtime }; + if (this.pendingDiskContents === data.disk.content) { + this.pendingDiskHash = data.disk.hash; + } + } this._conflict = new Conflict({ base: data.localText, ours: data.localText, @@ -3494,9 +3524,10 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { proxyDoc, Y.encodeStateVector(this.localDoc), ); + const previousText = this.localDoc.getText("contents").toString(); this.localDoc.transact(() => { Y.applyUpdate(this.localDoc!, diff, MACHINE_EDIT_ORIGIN); - this.syncFrontmatterToMap(); + this.syncFrontmatterToMap(previousText); }, MACHINE_EDIT_ORIGIN); } finally { proxyDoc.destroy(); @@ -3514,9 +3545,10 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { } else { // Normal user edit: apply directly to localDoc const ytext = this.localDoc.getText("contents"); + const previousText = ytext.toString(); this.localDoc.transact(() => { this.applyChangesToYText(ytext, e.changes); - this.syncFrontmatterToMap(); + this.syncFrontmatterToMap(previousText); }, this); } } @@ -3871,6 +3903,7 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { clean: false, localText, diskText, + disk, conflictRegions: computeTwoWayConflictRegions(localText, diskText), }; }, @@ -5682,7 +5715,7 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { // Mirror frontmatter to Y.Map atomically with the content change if (origin !== FRONTMATTER_MIRROR_ORIGIN) { - this.syncFrontmatterToMap(); + this.syncFrontmatterToMap(currentText); } }, origin ?? this); } @@ -6339,7 +6372,9 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { /** * Build a "correct" document by combining Y.Map frontmatter (LWW winners) - * with Y.Text body. Returns null if Y.Map is empty or YAML is unavailable. + * with Y.Text body. Returns null if YAML is unavailable or nothing + * usable remains to reconstruct (empty Y.Map, or no keys survive the + * text-owns-keys filter). */ private buildDocFromYMap(): string | null { if (!this.localDoc || !this._yaml) return null; @@ -6358,17 +6393,27 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { // apply only one side — producing duplicated frontmatter lines. const text = this.localDoc.getText("contents").toString(); const fm = this.parseFrontmatter(text); - const obj: Record = fm ? { ...fm.parsed } : {}; + // A missing block means the text owns an empty key set. An + // unrecoverable block may contain the user's in-progress input even + // though it cannot provide a safe key set yet. In either case, let + // the ordinary Y.Text delta reach the editor instead of rebuilding + // from stale map entries and resurrecting or discarding text. + if (!fm) return null; + const obj: Record = { ...fm.parsed }; + // The text owns the key set; the Y.Map owns values. Overlay LWW + // values only for keys the parsed frontmatter still carries — a + // key present only in the map was deleted from the text, and + // writing it back here is what resurrected deleted fields. + // Duplicate-key recovery has already produced a safe parsed key set. for (const [key, value] of ymap.entries()) { + if (!(key in obj)) continue; let parsed: any; try { parsed = JSON.parse(value as string); } catch { parsed = value; } obj[key] = parsed; } - for (const key of Object.keys(obj)) { - if (!ymap.has(key)) delete obj[key]; - } + if (Object.keys(obj).length === 0) return null; const yamlBody = this._yaml.stringify(obj); // Trailing `\n` on the canonical frontmatter is required so that @@ -6378,7 +6423,13 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { // dispatch — the shape producing `---\nhello` on disk for // live1/live2 butter.md after Properties toggles. const frontmatter = `---\n${yamlBody}---\n`; - const body = fm ? text.slice(fm.end) : text; + // The body is everything after the frontmatter REGION, located by + // the frontmatter-info helper — which finds the block whether or + // not its YAML parses. Falling back to the whole text on a parse + // failure would keep the broken block and prepend a fresh one on + // every dispatch, stacking blocks. + const info = this._yaml.getFrontMatterInfo(text); + const body = info.exists ? text.slice(info.contentStart) : text; return frontmatter + body; } @@ -6395,8 +6446,10 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { * end: `contentStart` — offset where the body begins * raw: the YAML body text (between the `---` delimiters) * parsed: parsed object, with on-disk key order preserved + * recovered: true when the block only parsed after de-duplicating + * repeated top-level keys (see below) */ - private parseFrontmatter(text: string): { start: number; end: number; parsed: Record; raw: string } | null { + private parseFrontmatter(text: string): { start: number; end: number; parsed: Record; raw: string; recovered: boolean } | null { if (!this._yaml) return null; const info = this._yaml.getFrontMatterInfo(text); @@ -6405,10 +6458,80 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { try { const parsed = this._yaml.parse(info.frontmatter); if (!parsed || typeof parsed !== "object") return null; - return { start: 0, end: info.contentStart, parsed, raw: info.frontmatter }; + return { start: 0, end: info.contentStart, parsed, raw: info.frontmatter, recovered: false }; } catch { - return null; + if (!flags().enableFrontmatterDuplicateRecovery) return null; + // Concurrent whole-line insertions can leave the same key on + // two lines, and duplicate keys make the block throw in the + // parser. Without a recovery path both mirror directions bail + // out on such a document forever. Retry with a last-wins + // de-duplication of top-level key lines so the document can + // converge back to a parseable state. + const deduped = this.dedupeTopLevelYamlKeys(info.frontmatter); + if (deduped === null) return null; + try { + const parsed = this._yaml.parse(deduped); + if (!parsed || typeof parsed !== "object") return null; + return { start: 0, end: info.contentStart, parsed, raw: info.frontmatter, recovered: true }; + } catch { + return null; + } + } + } + + /** + * Drop earlier occurrences of repeated top-level YAML keys, keeping + * the last one (matching the last-wins reading most parsers apply + * when they tolerate duplicates). A top-level entry is a column-0 + * `key:` line plus its indented continuation lines, so multi-line + * values move with their key. Returns null when no duplicate was + * found — the caller's parse failed for some other reason and this + * transformation cannot help. + */ + private dedupeTopLevelYamlKeys(yamlBody: string): string | null { + const lines = yamlBody.split("\n"); + type Entry = { key: string | null; lines: string[] }; + const entries: Entry[] = []; + let current: Entry | null = null; + // A column-0 line introducing a mapping key: everything before the + // first `:` that is followed by whitespace or end-of-line. Quoted + // keys are left untouched because a colon inside the quotes is data, + // not the key/value separator. + const keyLine = /^([^\s#'"-][^:]*):(?:\s|$)/; + for (const line of lines) { + const match = line.match(keyLine); + if (match) { + current = { key: match[1], lines: [line] }; + entries.push(current); + } else if (/^["']/.test(line)) { + // Keep a quoted top-level key as its own opaque entry. If it + // followed a duplicate plain key, attaching it as continuation + // text would delete it along with the earlier duplicate. + current = { key: null, lines: [line] }; + entries.push(current); + } else if (current) { + current.lines.push(line); + } else { + entries.push({ key: null, lines: [line] }); + } + } + + const seen = new Set(); + let droppedAny = false; + // Walk backwards so the last occurrence of each key survives. + for (let i = entries.length - 1; i >= 0; i--) { + const key = entries[i].key; + if (key === null) continue; + if (seen.has(key)) { + entries.splice(i, 1); + droppedAny = true; + } else { + seen.add(key); + } } + if (!droppedAny) return null; + + return entries.map((entry) => entry.lines.join("\n")).join("\n"); } /** @@ -6419,34 +6542,85 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { * enclosing transaction exists (e.g., initial seed), the caller is * responsible for wrapping in transact(). */ - private syncFrontmatterToMap(): void { + private seedFrontmatterMapFromCurrentText(allowBeforeProviderSync = false): void { + if (!this.localDoc || !this._yaml) return; + if (this.localDoc.getMap("frontmatter").size > 0) return; + if ( + !allowBeforeProviderSync && + !this._providerSynced && + !this._isProviderSynced() + ) return; + + this.localDoc.transact(() => { + this.syncFrontmatterToMap(); + }, this); + this._bridge.flushOutbound(); + } + + private syncFrontmatterToMap(previousText?: string): void { if (!this.localDoc || !this._yaml) return; const text = this.localDoc.getText("contents").toString(); const fm = this.parseFrontmatter(text); const ymap = this.localDoc.getMap("frontmatter"); - if (!fm) return; // Can't parse — don't touch the map - - const parsedKeys = Object.keys(fm.parsed); + if (!fm) { + // Distinguish a document with NO frontmatter block from one + // whose block would not parse even after recovery. The text + // owns key removal, so deleting the whole block prunes every + // key; a genuinely mangled block is left alone so the map + // keeps the last known-good values. + if ( + ymap.size > 0 && + (this._providerSynced || this._isProviderSynced()) && + !this._yaml.getFrontMatterInfo(text).exists + ) { + for (const key of [...ymap.keys()]) { + ymap.delete(key); + } + } + return; + } - // Safety: if the parsed frontmatter has fewer keys than the Y.Map, - // the Y.Text is likely corrupted (e.g., double --- delimiters causing - // a truncated parse). Skip the sync to avoid destroying Y.Map data. - if (ymap.size > 0 && parsedKeys.length < ymap.size) return; + let previousParsed: Record | null = null; + if (previousText !== undefined) { + const previousInfo = this._yaml.getFrontMatterInfo(previousText); + if (previousInfo.exists) { + const previousFm = this.parseFrontmatter(previousText); + // A malformed previous block has no safe structured delta. + if (!previousFm) return; + previousParsed = previousFm.parsed; + } else { + previousParsed = {}; + } + } - // Store each property value as a JSON string for faithful round-tripping. - // Y.Map uses LWW per key, so concurrent writes produce a clean winner. + // Store changed values as JSON strings for faithful round-tripping. + // Enrollment omits previousText to seed a full baseline. Edit paths + // provide it so unchanged stale values never become map writes. for (const [key, value] of Object.entries(fm.parsed)) { const serialized = JSON.stringify(value); - if (ymap.get(key) !== serialized) { + if ( + previousParsed === null || + !(key in previousParsed) || + JSON.stringify(previousParsed[key]) !== serialized + ) { ymap.set(key, serialized); } } - // Only delete keys when the parsed frontmatter is plausibly complete - for (const key of [...ymap.keys()]) { - if (!(key in fm.parsed)) { - ymap.delete(key); + // The text owns key removal: a key deleted from the frontmatter + // must leave the map, or the next reconstruction resurrects it. + // Prune only while the provider is synced — before first sync the + // local text may simply predate map entries written by peers, and + // a delete issued from stale text would destroy them for everyone. + // An unpruned stale key is inert (reconstruction never reintroduces + // keys the text lacks) and is pruned at the next reconciliation by + // repairFrontmatterFromMap instead. + if (this._providerSynced || this._isProviderSynced()) { + for (const key of [...ymap.keys()]) { + if (!(key in fm.parsed)) { + ymap.delete(key); + } } } } @@ -6470,38 +6644,54 @@ export class MergeHSM implements MachineHSM, SyncBridgeHost { const text = this.localDoc.getText("contents").toString(); const fm = this.parseFrontmatter(text); - if (!fm) return; // No frontmatter block to repair + if (!fm) return; // No parseable frontmatter to repair against // Mirror Obsidian's processFrontMatter: start from the parsed - // frontmatter (preserves on-disk key order) and mutate in place - // using Y.Map's LWW values. Keys absent from Y.Map are dropped. - // Keys present in Y.Map but absent from the parsed frontmatter - // are appended. This keeps Obsidian's writes and our repairs - // emitting the same key order so DMP only sees content-level - // changes, never reorders. + // frontmatter (preserves on-disk key order) and overlay Y.Map's + // LWW values in place. This keeps Obsidian's writes and our + // repairs emitting the same key order so DMP only sees + // content-level changes, never reorders. + // + // The text owns the key set. A map key the parsed frontmatter no + // longer carries was deleted from the text, so it is pruned from + // the map here rather than written back: re-inserting the line is + // what resurrected deleted fields, and two clients re-inserting + // the same line each contribute a copy the merge keeps, while two + // clients pruning the same map entry converge to one state. const obj: Record = { ...fm.parsed }; + const staleKeys: string[] = []; for (const [key, value] of ymap.entries()) { + if (!(key in obj)) { + staleKeys.push(key); + continue; + } let parsed: any; try { parsed = JSON.parse(value as string); } catch { parsed = value; } obj[key] = parsed; } - for (const key of Object.keys(obj)) { - if (!ymap.has(key)) delete obj[key]; - } - // Corruption check: values differ, or the set of keys differs. - let corrupted = false; - const parsedKeys = Object.keys(fm.parsed); - const objKeys = Object.keys(obj); - if (parsedKeys.length !== objKeys.length) { - corrupted = true; - } else { - for (const key of objKeys) { - if (JSON.stringify(fm.parsed[key]) !== JSON.stringify(obj[key])) { - corrupted = true; - break; + if (staleKeys.length > 0) { + this.crdtLog( + `frontmatter mirror: pruning ${staleKeys.length} map key(s) deleted from the text`, + ); + this.localDoc.transact(() => { + for (const key of staleKeys) { + ymap.delete(key); } + }, FRONTMATTER_MIRROR_ORIGIN); + } + + // Corruption check: a map value differs from what the text + // carries. A block that only parsed after de-duplication is also + // rewritten — the rewrite emits the canonical single-line-per-key + // block, and when the values already agree that diff is pure + // deletion, which concurrent identical repairs apply idempotently. + let corrupted = fm.recovered; + for (const key of Object.keys(obj)) { + if (JSON.stringify(fm.parsed[key]) !== JSON.stringify(obj[key])) { + corrupted = true; + break; } } diff --git a/src/merge-hsm/MergeManager.ts b/src/merge-hsm/MergeManager.ts index 97b95dd9..2232cfad 100644 --- a/src/merge-hsm/MergeManager.ts +++ b/src/merge-hsm/MergeManager.ts @@ -1188,6 +1188,11 @@ export class MergeManager { this._updateWakeQueueMetrics(); } + /** Bulk-loaded record metadata for a document or managed file. */ + getPersistedStateMeta(guid: string): PersistedStateMeta | undefined { + return this._stateMetaCache.get(guid) ?? this._managedMetaCache.get(guid); + } + /** Bulk-loaded record metadata for a managed file (cold-start input). */ getManagedMeta(guid: string): PersistedStateMeta | undefined { return this._managedMetaCache.get(guid); diff --git a/src/merge-hsm/integration/HSMEditorPlugin.ts b/src/merge-hsm/integration/HSMEditorPlugin.ts index 88688ff3..83a7f672 100644 --- a/src/merge-hsm/integration/HSMEditorPlugin.ts +++ b/src/merge-hsm/integration/HSMEditorPlugin.ts @@ -83,9 +83,24 @@ export class HSMEditorPluginValue implements PluginValue { * integration's validity check reports the editor not-ready until then. */ private bornAttachedRenderPending = false; + /** + * Whether this EditorView has been identified as an embedded sub-editor: + * an editor another editor spawns inside itself over the same file, like + * the per-cell editor Obsidian's Live Preview table widget creates when a + * table cell is edited. A sub-editor inherits the host view's + * editorInfoField, so file identity, document resolution, and + * source-view DOM ancestry all match the host — but its buffer holds only + * a fragment of the note, and the spawning machinery persists whatever is + * dispatched into it back into the note as a user edit. Binding one to + * the merge machinery would render the full document into a fragment + * buffer (and the widget would then write that full document into one + * cell of the real note) and would feed fragment text back as document + * content. Sticky: once detected, this instance is permanently inert. + */ + private subEditor = false; private bindingEpoch = 0; private lastInitializationRetry: - | { file: TFile | null; live: boolean } + | { file: TFile | null; live: boolean; owner: EditorView | null } | null = null; private log: (...args: unknown[]) => void; private debug: (...args: unknown[]) => void; @@ -226,6 +241,52 @@ export class HSMEditorPluginValue implements PluginValue { ); } + /** + * Detect an embedded sub-editor by its container: the Live Preview table + * widget mounts the per-cell editor it spawns inside a + * `.table-cell-wrapper` element. Detection is sticky and fully inerts + * this instance — the widget forwards every transaction it does not + * recognize into the host note, so nothing may ever be dispatched into + * such an editor. The wrapper is only observable once the editor's DOM is + * attached, so a negative answer means "not detected", never "proven to + * be a view's own editor"; the owner-identity check in probeBornAttached + * covers the window before the DOM is attached. + */ + private probeSubEditor(): boolean { + if (this.subEditor) return true; + if (!this.editor.dom.closest(".table-cell-wrapper")) return false; + this.subEditor = true; + this.log("Refusing to bind an embedded table-cell editor"); + if (this.cm6Integration) { + this.cm6Integration.destroy(); + this.cm6Integration = null; + } + this.clearPendingEdits(); + this.document = null; + return true; + } + + /** + * The EditorView that this editor's owning view considers its editor, + * when resolvable. An embedded sub-editor inherits its host's + * editorInfoField, so the field resolves to the HOST view and names the + * host's EditorView — a resolvable owner editor that is not this view + * identifies a sub-editor even before its DOM is attached. Returns null + * when the owner's editor is not resolvable (a view still under + * construction has not assigned its editor yet). + */ + private ownerEditorView(): EditorView | null { + try { + const fileInfo = this.editor.state.field(editorInfoField, false); + const ownerEditor = fileInfo?.editor as + | { cm?: EditorView } + | undefined; + return ownerEditor?.cm ?? null; + } catch { + return null; + } + } + /** * Decide, once per document binding, whether this view was created while * the document's merge machinery was already active and holding the editor @@ -237,10 +298,26 @@ export class HSMEditorPluginValue implements PluginValue { */ private probeBornAttached(): boolean { if (this.bornAttached === null) { + if (this.probeSubEditor()) return false; const fileInfo = this.editor.state.field(editorInfoField, false); if (fileInfo?.file) { const doc = this.resolveCurrentDocument(); if (doc) { + // Born-attached is the one bind that skips the positive identity + // check against the view registry, so the probe itself must prove + // this view is a view's own editor. File identity, the info + // field, and source-view ancestry cannot: an embedded sub-editor + // (a table-cell editor) matches its host on all three. The owning + // view names its editor; a resolvable owner editor that is not + // this view is a sub-editor. Unresolvable leaves the decision + // open rather than deciding false: a view under construction + // assigns its editor only after extensions instantiate, and an + // in-place editor replacement names the outgoing editor until the + // owner adopts the new one. + const ownerCm = this.ownerEditorView(); + if (ownerCm !== null && ownerCm !== this.editor) { + return false; + } const sourceView = this.editor.dom.closest(".markdown-source-view"); const embed = !!sourceView?.classList.contains("mod-inside-iframe"); this.bornAttached = @@ -260,6 +337,7 @@ export class HSMEditorPluginValue implements PluginValue { initializeIfReady(): boolean { if (this.cm6Integration) return true; if (this.destroyed) return false; + if (this.probeSubEditor()) return false; const connectionManager = getConnectionManager(this.editor); if (!connectionManager) return false; @@ -339,10 +417,6 @@ export class HSMEditorPluginValue implements PluginValue { this.debug(`Initialized for ${this.document.guid} (embed: ${this.embed})`); const currentText = this.editor.state.doc.toString(); - hsm.attachEditorView( - { getViewData: () => this.editor.state.doc.toString() }, - currentText, - ); if (bornAttached) { // Born attached: render the authoritative document text directly and @@ -354,6 +428,11 @@ export class HSMEditorPluginValue implements PluginValue { return true; } + hsm.attachEditorView( + { getViewData: () => this.editor.state.doc.toString() }, + currentText, + ); + if (this.pendingEdits.length === 0) { hsm.bootstrapEditorView( this.cm6Integration.viewId, @@ -393,6 +472,15 @@ export class HSMEditorPluginValue implements PluginValue { // must not clear or consume the next binding's pending input. if (epoch !== this.bindingEpoch || integration !== this.cm6Integration) return; this.bornAttachedRenderPending = false; + // By render time the editor's DOM is attached: a sub-editor that + // escaped the construction-time probes is detectable here, before + // anything is dispatched into it. The probe tears the binding down. + if (this.probeSubEditor()) { + this.log( + `Aborting born-attached render for ${expectedGuid}: embedded sub-editor`, + ); + return; + } // Take over all pre-render input at fire time: the buffered layer // (including anything routed here during the render-pending window) // and any restores replacement transactions extracted before the @@ -432,6 +520,16 @@ export class HSMEditorPluginValue implements PluginValue { const localText = localDoc.getText("contents").toString(); const currentText = this.editor.state.doc.toString(); + // Do not replace the HSM's legitimate sibling view reference until + // every late born-attached check has passed. In particular, a nested + // editor whose container becomes observable only after construction + // must abort above without leaving its fragment buffer as the HSM's + // source of editor truth. + hsm.attachEditorView( + { getViewData: () => this.editor.state.doc.toString() }, + currentText, + ); + // Replace whatever the buffer holds — stale disk load, a save echo, // pre-bind typing — with the document text. if (currentText !== localText) { @@ -517,7 +615,7 @@ export class HSMEditorPluginValue implements PluginValue { * This is called on every editor state change. */ update(update: ViewUpdate): void { - if (this.destroyed) return; + if (this.destroyed || this.subEditor) return; if (update.docChanged) { this.lastInitializationRetry = null; } @@ -590,9 +688,15 @@ export class HSMEditorPluginValue implements PluginValue { if (!this.cm6Integration) { const file = this.editor.state.field(editorInfoField, false)?.file ?? null; const live = this.isLiveEditor(); + const owner = this.ownerEditorView(); const prior = this.lastInitializationRetry; - if (!prior || prior.file !== file || prior.live !== live) { - this.lastInitializationRetry = { file, live }; + if ( + !prior || + prior.file !== file || + prior.live !== live || + prior.owner !== owner + ) { + this.lastInitializationRetry = { file, live, owner }; this.initializeIfReady(); } } diff --git a/src/merge-hsm/machine-definition.ts b/src/merge-hsm/machine-definition.ts index eda06eac..8ffa6898 100644 --- a/src/merge-hsm/machine-definition.ts +++ b/src/merge-hsm/machine-definition.ts @@ -715,7 +715,7 @@ export const MACHINE: MachineDefinition = { canPersistFullLca: true, canUseRemoteDoc: true, }, - entry: ['replayAccumulatedEvents', 'mergeRemoteToLocal', 'repairFrontmatter', 'assertConvergence', 'reconcileForkInActive'], + entry: ['replayAccumulatedEvents', 'mergeRemoteToLocal', 'seedFrontmatterMap', 'repairFrontmatter', 'assertConvergence', 'reconcileForkInActive'], on: { CM6_CHANGE: { target: 'active.tracking', actions: ['applyCM6ToLocalDoc'] }, REMOTE_DOC_UPDATED: { target: 'active.tracking', actions: ['mergeRemoteToLocal', 'repairFrontmatter'] }, @@ -732,7 +732,7 @@ export const MACHINE: MachineDefinition = { DISK_CHANGED: { target: 'active.tracking', actions: ['storeDiskMetadataOnly'] }, CONNECTED: { target: 'active.tracking', actions: ['flushPendingToRemote', 'mergeRemoteToLocal'] }, DISCONNECTED: { target: 'active.tracking', actions: ['setOffline'] }, - PROVIDER_SYNCED: { target: 'active.tracking', actions: ['markProviderSynced', 'reconcileForkInActive'] }, + PROVIDER_SYNCED: { target: 'active.tracking', actions: ['markProviderSynced', 'mergeRemoteToLocal', 'seedFrontmatterMap', 'reconcileForkInActive'] }, MERGE_CONFLICT: { target: 'active.conflict.bannerShown', actions: ['storeConflictData'] }, RELEASE_LOCK: { target: 'unloading', actions: ['beginReleaseLock'] }, UNLOAD: { target: 'unloading', actions: ['beginUnload'] }, diff --git a/src/merge-hsm/testing/createTestHSM.ts b/src/merge-hsm/testing/createTestHSM.ts index 16f316f2..b20104be 100644 --- a/src/merge-hsm/testing/createTestHSM.ts +++ b/src/merge-hsm/testing/createTestHSM.ts @@ -397,6 +397,17 @@ export async function createTestHSM( initializedAfterSync = true; return true; }, + async initializeWithContent(contentLoader, fieldName = "contents") { + if (hadContentAtSync || initializedAfterSync) return false; + const { content } = await contentLoader(); + doc.transact(() => { + const header = doc.getMap("relay"); + if (!header.has("v")) header.set("v", 0); + doc.getText(fieldName).insert(0, content); + }); + initializedAfterSync = true; + return true; + }, opCapture: null as OpCapture | null, }; diff --git a/src/y-codemirror.next/UserAttributionPlugin.ts b/src/y-codemirror.next/UserAttributionPlugin.ts index b6d4744c..bd093079 100644 --- a/src/y-codemirror.next/UserAttributionPlugin.ts +++ b/src/y-codemirror.next/UserAttributionPlugin.ts @@ -34,7 +34,34 @@ export const attributionFilterField = StateField.define({ }); export const userAttributionTheme = EditorView.baseTheme({ - ".cm-attribution": {}, + ".cm-attribution": { + position: "relative", + }, + // Author name badge, rendered purely in CSS from the mark's data-author + // attribute so it appears the instant the pointer enters — a native title + // attribute would wait out the browser's fixed tooltip delay. Styled to + // match the remote-caret name label (.cm-ySelectionInfo). Only exists + // while attribution decorations do, so it costs nothing when the mode is + // off and nothing outside hover while it is on. + ".cm-attribution:hover::after": { + content: "attr(data-author)", + position: "absolute", + top: "-1.05em", + left: "-1px", + fontSize: ".75em", + fontFamily: "serif", + fontStyle: "normal", + fontWeight: "normal", + lineHeight: "normal", + userSelect: "none", + pointerEvents: "none", + color: "white", + paddingLeft: "2px", + paddingRight: "2px", + zIndex: 101, + backgroundColor: "var(--cm-attribution-color)", + whiteSpace: "nowrap", + }, }); function filterIncludes( @@ -50,7 +77,7 @@ function readAttributionFilter(state: EditorState): AttributionFilter { return state.field(attributionFilterField, false) ?? null; } -class UserAttributionPluginValue { +export class UserAttributionPluginValue { decorations: DecorationSet = Decoration.none; editor: EditorView; private destroyed = false; @@ -177,8 +204,9 @@ class UserAttributionPluginValue { ranges.push( Decoration.mark({ attributes: { - style: `background-color: ${color.light}; border-bottom: 1px solid ${color.color}`, - title: `Written by ${display}`, + style: `background-color: ${color.light}; border-bottom: 1px solid ${color.color}; --cm-attribution-color: ${color.color}`, + "data-author": display, + "aria-description": `Written by ${display}`, }, class: "cm-attribution", }).range(from, to),