diff --git a/desktop/src/features/sidebar/lib/wholeBlobSync.shared.test.mjs b/desktop/src/features/sidebar/lib/wholeBlobSync.shared.test.mjs index f6efbc33dd..99338227e2 100644 --- a/desktop/src/features/sidebar/lib/wholeBlobSync.shared.test.mjs +++ b/desktop/src/features/sidebar/lib/wholeBlobSync.shared.test.mjs @@ -839,3 +839,242 @@ test("P2b decrypt gap: watermark advances synchronously; lastRemoteHead advances mock.reset(); } }); + +// ── P2a queue-until-bootstrap regressions ──────────────────────────────────── +// +// Three causal regressions covering the queue-until-bootstrap contract. +// +// Shared mutation target A (tests 1 and 2): removing the `bootstrapStarted` +// guard from `publish()` lets the debounce timer fire while bootstrap is still +// in-flight. `publishBaseline` is `{0,""}` at that point, so +// `remoteAdvancedSince(head, {0,""})` = true and `fetchOwnBlobBeforePublish` +// adopts the edit away — exactly the Kalvin P2a click-loss. +// +// Shared mutation target B (test 3): removing the `bootstrapFailed` exception +// from `fetchOwnBlobBeforePublish` causes a hard-adopt against `{0,""}` when +// bootstrap failed, dropping the queued edit even though no baseline was ever +// established. + +// Test 1 (load-bearing): fresh device (watermark=0), relay HAS a pre-existing +// head, user clicks during unresolved bootstrap → bootstrap succeeds → edit +// publishes above the relay head, never adopted away. +// Failing mutation: remove `this.bootstrapStarted &&` from the publish() guard +// (i.e. revert to unconditional timer scheduling) → debounce fires against +// {0,""} baseline → remoteAdvancedSince returns true → adopt fires instead. +test("P2a queue-until-bootstrap: click during unresolved bootstrap on fresh device publishes above the relay head, never adopted", async () => { + let releaseBootstrap = null; + let publishCalls = 0; + const { fireDelay, restore } = makeTimerBed(); + const remoteStore = makeStore([{ id: "remote", name: "Remote", order: 0 }]); + const tauri = installEchoTauri("pk-p2a-1"); + + // storedHead tracks what the relay retains. Initially the pre-existing head; + // updated to our published event once we publish successfully. + let storedHead = [tauri.mintHead(remoteStore, 200, "evt-relay-head")]; + + // Fetch call 1 (bootstrap): blocks until released, returns the pre-existing + // relay head. Subsequent calls (pre-publish fetch, confirmation) return + // storedHead — which after our publish will be our own event. + let fetchCalls = 0; + mock.method(relayClient, "fetchEvents", () => { + fetchCalls++; + if (fetchCalls === 1) + return new Promise((res) => { + releaseBootstrap = () => res(storedHead); + }); + return Promise.resolve(storedHead); + }); + mock.method(relayClient, "publishEvent", (e) => { + publishCalls++; + storedHead = [e]; // relay retains our event + return Promise.resolve(); + }); + + try { + const manager = new SyntheticWholeBlobManager("pk-p2a-1", RELAY); + const adopted = []; + manager.setOnRemoteAdopted((r) => adopted.push(r)); + + // Start bootstrap (blocked on fetch call 1). + const bootstrapPromise = manager.bootstrap(nonEmptyStore()); + + // User clicks during unresolved bootstrap. With the fix: no timer fires yet. + // Without the fix (mutation): timer fires immediately against {0,""} baseline. + manager.publishSections( + makeStore([{ id: "click", name: "Click", order: 0 }]), + ); + + // Release bootstrap → head is recorded (200,"evt-relay-head"), bootstrapResolved=true, + // releaseDeferred() re-freezes publishBaseline to {200,"evt-relay-head"} and + // schedules the 2s debounce. + while (releaseBootstrap === null) await Promise.resolve(); + releaseBootstrap(); + await bootstrapPromise; + for (let i = 0; i < 50; i++) await Promise.resolve(); + + // Fire the debounce released by bootstrap. + await fireDelay(2000); + for (let i = 0; i < 100; i++) await Promise.resolve(); + + // The edit must have published. publishBaseline was {200,"evt-relay-head"}; + // pre-publish fetch returns the same head → remoteAdvancedSince = false → publish. + // After our publish, storedHead = our event; confirmation fetch confirms us. + // Without the fix: baseline was {0,""} → remoteAdvancedSince = true → adopt. + assert.equal(adopted.length, 0, "click must not be adopted away"); + assert.ok(publishCalls >= 1, "click must publish above the relay head"); + assert.equal( + manager.getPendingStore(), + null, + "pending cleared after publish", + ); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + +// Test 2: click during unresolved bootstrap, bootstrap succeeds, then a peer +// posts a genuinely newer head BEFORE the debounce fires → edit adopts (normal LWW). +// Failing mutation: same as test 1 — without the guard, the debounce fires during +// bootstrap with baseline {0,""}. The pre-publish fetch returns a {0,""}-relative +// advance but for the wrong reason (before the bootstrap head was established). +// With the fix, publishBaseline = {200,"evt-boot"} and the peer head at ts=400 +// is a genuine advance → adopt fires correctly. +test("P2a queue-until-bootstrap: genuinely newer peer head that arrives after bootstrap adopt via normal LWW", async () => { + let releaseBootstrap = null; + let publishCalls = 0; + const { fireDelay, restore } = makeTimerBed(); + const bootstrapStore = makeStore([{ id: "boot", name: "Boot", order: 0 }]); + const peerStore = makeStore([{ id: "peer", name: "Peer", order: 0 }]); + const tauri = installEchoTauri("pk-p2a-2"); + + const bootHead = tauri.mintHead(bootstrapStore, 200, "evt-boot"); + const peerHead = tauri.mintHead(peerStore, 400, "evt-peer"); + + // Bootstrap returns the boot head. Pre-publish fetch returns the peer-newer + // head (simulating a peer publishing between bootstrap and doPublish). + let fetchCalls = 0; + mock.method(relayClient, "fetchEvents", () => { + fetchCalls++; + if (fetchCalls === 1) + return new Promise((res) => { + releaseBootstrap = () => res([bootHead]); + }); + return Promise.resolve([peerHead]); + }); + mock.method(relayClient, "publishEvent", () => { + publishCalls++; + return Promise.resolve(); + }); + + try { + const manager = new SyntheticWholeBlobManager("pk-p2a-2", RELAY); + const adopted = []; + manager.setOnRemoteAdopted((r) => adopted.push(r)); + + const bootstrapPromise = manager.bootstrap(nonEmptyStore()); + + // Click during unresolved bootstrap. + manager.publishSections( + makeStore([{ id: "click-2", name: "Click", order: 0 }]), + ); + + while (releaseBootstrap === null) await Promise.resolve(); + releaseBootstrap(); + await bootstrapPromise; + // After bootstrap: publishBaseline = {200,"evt-boot"} (re-frozen by releaseDeferred). + for (let i = 0; i < 50; i++) await Promise.resolve(); + + await fireDelay(2000); + for (let i = 0; i < 100; i++) await Promise.resolve(); + + // Pre-publish fetch returns {400,"evt-peer"}: remoteAdvancedSince({400,...},{200,...})=true + // → adopt fires correctly. Normal LWW: peer truly posted after bootstrap baseline. + assert.equal( + publishCalls, + 0, + "edit must not publish over the newer peer head", + ); + assert.equal( + adopted.length, + 1, + "newer peer head must be adopted (normal LWW)", + ); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); + +// Test 3 (Kalvin's original P2a sequence): bootstrap FAILS, user clicks, +// pre-publish fetch finds a pre-existing head → edit publishes above it. +// Failing mutation: remove the `this.bootstrapFailed &&` check from +// fetchOwnBlobBeforePublish — the hard-adopt against {0,""} baseline fires. +test("P2a failed bootstrap: click after failed bootstrap publishes above the relay head, never adopts against empty baseline", async () => { + let publishCalls = 0; + const { fireDelay, restore } = makeTimerBed(); + const relayStore = makeStore([{ id: "relay", name: "Relay", order: 0 }]); + const tauri = installEchoTauri("pk-p2a-3"); + + // storedHead starts as the pre-existing relay head; updated to our event on publish. + let storedHead = [tauri.mintHead(relayStore, 100, "evt-existing")]; + + // Bootstrap fetch: call 1 rejects. Subsequent calls: return storedHead. + let fetchCalls = 0; + mock.method(relayClient, "fetchEvents", () => { + fetchCalls++; + if (fetchCalls === 1) return Promise.reject(new Error("relay timeout")); + return Promise.resolve(storedHead); + }); + mock.method(relayClient, "publishEvent", (e) => { + publishCalls++; + storedHead = [e]; // relay retains our event + return Promise.resolve(); + }); + + try { + const manager = new SyntheticWholeBlobManager("pk-p2a-3", RELAY); + const adopted = []; + manager.setOnRemoteAdopted((r) => adopted.push(r)); + + // Bootstrap fails: bootstrapFailed=true, bootstrapResolved=true. + // No releaseDeferred edit to schedule (nothing was queued during bootstrap). + await manager.bootstrap(nonEmptyStore()); + + // User clicks after failed bootstrap — publish() sees bootstrapResolved=true + // so it schedules immediately with publishBaseline={0,""} (lastRemoteHead + // was never advanced because bootstrap failed). + manager.publishSections( + makeStore([{ id: "click-3", name: "Click", order: 0 }]), + ); + + await fireDelay(2000); + for (let i = 0; i < 100; i++) await Promise.resolve(); + + // Pre-publish fetch returns {100,"evt-existing"}: remoteAdvancedSince(true). + // bootstrapFailed+empty-baseline exception: fold the head in and publish + // above it instead of adopting. After publish, storedHead = our event; + // confirmation confirms us. + // Without the fix: hard-adopt fires, publishCalls=0, adopted.length=1. + assert.equal( + adopted.length, + 0, + "click must not be adopted against empty baseline", + ); + assert.ok(publishCalls >= 1, "click must publish above the relay head"); + assert.equal( + manager.getPendingStore(), + null, + "pending cleared after publish", + ); + manager.destroy(); + } finally { + tauri.restore(); + restore(); + mock.reset(); + } +}); diff --git a/desktop/src/features/sidebar/lib/wholeBlobSyncManager.ts b/desktop/src/features/sidebar/lib/wholeBlobSyncManager.ts index f56a89e05b..9a912f1b76 100644 --- a/desktop/src/features/sidebar/lib/wholeBlobSyncManager.ts +++ b/desktop/src/features/sidebar/lib/wholeBlobSyncManager.ts @@ -198,6 +198,25 @@ export class WholeBlobSyncManager { // touching shared manager state, so a stale generation can never sign or // publish after a newer edit exists. private publishInFlight = false; + // Whether bootstrap() has been called (started) and whether it has completed. + // publish() defers the debounce timer only when bootstrap was started but has + // not yet resolved. When bootstrap() is never called (e.g. standalone tests + // or managers used without the standard hook lifecycle), both flags stay false + // and publish() behaves as before — schedules the timer immediately. + // + // The P2a queue-until-bootstrap contract: no publish debounce fires between + // bootstrap starting and bootstrap resolving, so a click during an unresolved + // bootstrap is never silently adopted away against an empty `{0,""}` baseline. + private bootstrapStarted = false; + private bootstrapResolved = false; + // True when bootstrap completed with a failed fetch — no remote head was ever + // established. Used by the P2a failed-bootstrap exception in + // fetchOwnBlobBeforePublish so an edit whose baseline is {0,""} because + // bootstrap failed still publishes ABOVE the first head it discovers rather + // than adopting it away. (For a successful bootstrap, releaseDeferred re-freezes + // publishBaseline to lastRemoteHead, so the baseline is not {0,""} at doPublish + // and this flag is never consulted.) + private bootstrapFailed = false; // Event ids we signed and sent to the relay but whose ACK never arrived (the // publish promise rejected as a timeout/socket error after the frame left). // The relay MAY have accepted such a write, so if a later cycle's pre-publish @@ -318,10 +337,12 @@ export class WholeBlobSyncManager { * would be published over instead of adopted (Carl P1). Waking the existing * generation keeps the frozen baseline, so the debounced cycle's pre-publish * check still adopts a genuinely-advanced remote. No-op when nothing is - * pending. + * pending or when bootstrap has not yet resolved (the deferred-release path + * handles that case once bootstrap completes). */ retryPendingPublish(): void { if (this.pendingStore === null) return; + if (this.bootstrapStarted && !this.bootstrapResolved) return; this.cancelPendingPublish(); this.startCycle(); } @@ -331,6 +352,42 @@ export class WholeBlobSyncManager { return this.pendingStore !== null; } + /** + * Release any edit that was deferred until bootstrap resolved. Called by + * bootstrap() after it sets bootstrapResolved=true. If a pending edit exists + * with no timer or in-flight cycle, re-freeze publishBaseline from the + * now-established lastRemoteHead and schedule the normal 2s debounce. + * + * Re-freezing the baseline here (rather than leaving it at the {0,""} it had + * when publish() was called during unresolved bootstrap) is the P2a fix for + * the success path: the edit's comparison point becomes the head that + * bootstrap just established, so the pre-publish fetch for the same head + * returns "not advanced" and the edit publishes above it correctly. A peer + * head that arrives BETWEEN bootstrap resolution and doPublish is correctly + * seen as a genuine advance (its timestamp exceeds the just-frozen baseline) + * and is adopted per normal LWW. For a failed bootstrap, lastRemoteHead stays + * {0,""} and the bootstrapFailed flag enables the doPublish-time exception. + */ + private releaseDeferred(): void { + if ( + !this.destroyed && + this.pendingStore !== null && + this.debounceTimer === null && + this.retryTimer === null && + !this.publishInFlight + ) { + // Re-freeze the baseline to the head now known from bootstrap. For a + // successful bootstrap this is the fetched head; for a failed bootstrap + // lastRemoteHead is still {0,""} (the bootstrapFailed exception handles + // that case separately in fetchOwnBlobBeforePublish). + this.publishBaseline = { ...this.lastRemoteHead }; + this.debounceTimer = window.setTimeout(() => { + this.debounceTimer = null; + this.startCycle(); + }, DEBOUNCE_MS); + } + } + /** * When the current `publishBaseline` was frozen against one of our own * attempted (but non-retained) event ids and `remote` is the same-second @@ -441,6 +498,7 @@ export class WholeBlobSyncManager { const durable = this.config.writeOutbox(this.pubkey, store, this.relayUrl); if (this.debounceTimer !== null) { window.clearTimeout(this.debounceTimer); + this.debounceTimer = null; } // A fresh edit supersedes any retry scheduled for the previous generation. if (this.retryTimer !== null) { @@ -448,10 +506,19 @@ export class WholeBlobSyncManager { this.retryTimer = null; } this.retryDelayMs = RETRY_BASE_MS; - this.debounceTimer = window.setTimeout(() => { - this.debounceTimer = null; - this.startCycle(); - }, DEBOUNCE_MS); + // Do not start the debounce timer until bootstrap resolves. The intent is + // durably held (outbox write + pendingStore set above) — it will be released + // by releaseDeferred() once bootstrap completes. This prevents doPublish + // from running against the empty `{0,""}` baseline and adopting the edit + // away on a fresh device whose bootstrap fetch hasn't returned yet. + // When bootstrap() has not been called at all, bootstrapStarted is false + // and the timer is scheduled immediately (preserving prior behavior). + if (!this.bootstrapStarted || this.bootstrapResolved) { + this.debounceTimer = window.setTimeout(() => { + this.debounceTimer = null; + this.startCycle(); + }, DEBOUNCE_MS); + } return durable; } @@ -536,6 +603,27 @@ export class WholeBlobSyncManager { if (this.foldSupersedingAttemptWinner(remote)) { return { kind: "publish", store, fetchedRemote: remote }; } + // Failed-bootstrap exception (P2a): the edit was queued while bootstrap + // was in-flight and bootstrap subsequently FAILED — no head was ever + // fetched, so publishBaseline and lastRemoteHead were never advanced. + // releaseDeferred() re-freezes publishBaseline from lastRemoteHead, but + // when bootstrap failed that is still {0,""}. Any head the relay returns + // here is the TRUE first established baseline; publish above it rather + // than adopting it away. (When bootstrap SUCCEEDS, releaseDeferred sets + // publishBaseline to the fetched head before scheduling the timer, so + // this branch is not reached for successful bootstraps whose head was + // already known when the debounce fired.) + if ( + this.bootstrapFailed && + this.publishBaseline.createdAt === 0 && + this.publishBaseline.eventId === "" + ) { + this.publishBaseline = canonicalMax(this.publishBaseline, { + createdAt: remote.createdAt, + eventId: remote.eventId, + }); + return { kind: "publish", store, fetchedRemote: remote }; + } return { kind: "adopt", remote }; } return { kind: "publish", store, fetchedRemote: remote }; @@ -794,16 +882,37 @@ export class WholeBlobSyncManager { /** * Fetches the remote blob on first mount, records the remote head, and * delegates the seed/hold/apply-remote decision to `runBootstrap`. + * + * Sets `bootstrapResolved` before returning so the hook's `.then()` callback + * (which may call publish() for outbox replay) sees the flag as true and + * schedules the debounce timer immediately. Then releases any edit that was + * queued BEFORE the hook's callback runs (callers who called publish() from + * another code path during the async fetch). */ async bootstrap(localStore: S) { + // Set bootstrapStarted synchronously (before the first await) so any + // publish() call that races the async fetch defers its debounce timer. + this.bootstrapStarted = true; const fetchResult = await this.fetchRemoteBlob(); - return runBootstrap({ + // Track whether the bootstrap fetch failed so the P2a failed-bootstrap + // exception in fetchOwnBlobBeforePublish can fire when appropriate. + if (fetchResult.status === "failed") { + this.bootstrapFailed = true; + } + const result = runBootstrap({ fetchResult, lastHead: this.lastRemoteCreatedAt, localStore, isLocalNonEmpty: this.config.isLocalNonEmpty, publishFn: (s) => this.publish(s), }); + // Mark bootstrap resolved BEFORE returning. The hook's .then() runs + // synchronously on the resolved promise, so any publish() it calls will + // already see bootstrapResolved=true and schedule the debounce normally. + // releaseDeferred() covers edits queued via other paths during the fetch. + this.bootstrapResolved = true; + this.releaseDeferred(); + return result; } destroy(): void {