fix(sidebar-sync): preserve publishBaseline on superseding click (Carl P2)

wholeBlobSyncManager.startPendingEdit() unconditionally reseeded
publishBaseline from mutable lastRemoteHead on every non-replay call.
Sequence: A1 click pending → live H102 suppressed by hasPendingEdit →
A2 second click → lastRemoteHead (=H102) folded into baseline →
preflight saw no advance → A2 silently overwrote H102-only changes.
Contract violation per JSDoc :51-55/:163-174.

Fix: reseed publishBaseline only when (a) starting a fresh pending
sequence (wasIdle) or (b) lastRemoteHead holds our OWN in-flight
published attempt (lastIsOwnAttempt: eventId ∈ ambiguousAttemptIds).
Case (b) is required so foldSupersedingAttemptWinner can repair the
baseline on a same-second collision. A suppressed live remote head is
never in ambiguousAttemptIds → lastIsOwnAttempt=false → baseline is
preserved across all pending phases (debounce, preflight, publish,
confirmation wait).

Replay path (isRestoredReplay) semantics unchanged.

New tests: wholeBlobSyncCarlP2SecondClick.shared.test.mjs, Variant A
(debounce-window) and Variant B (in-flight), wired into both lane
test files. Required mutations: timer-phase revert turns Variant B
red; unconditional-reseed revert turns all four red. Same-second
collision test remains green under both mutations.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
Duncan
2026-09-03 11:21:32 -04:00
parent 49902fd4bc
commit ffda3579a5
4 changed files with 334 additions and 13 deletions

View File

@@ -13,6 +13,7 @@ import {
runWholeBlobP2a1HookSuite,
runWholeBlobP2bSuite,
} from "./wholeBlobSyncCarl.shared.test.mjs";
import { runWholeBlobCarlP2SecondClickSuite } from "./wholeBlobSyncCarlP2SecondClick.shared.test.mjs";
const { act, cleanup, renderHook } = await import("@testing-library/react");
const { relayClient } = await import("@/shared/api/relayClient");
@@ -384,3 +385,15 @@ runWholeBlobP2a1HookSuite({
);
},
});
runWholeBlobCarlP2SecondClickSuite({
label: "sections",
useHook: useChannelSections,
makeEdit1: (r) => r.createSection("A1-Section"),
makeEdit2: (r) => r.createSection("A2-Section"),
makeRemoteStore: () => ({
version: 1,
sections: [{ id: "remote", name: "Remote", order: 0 }],
assignments: {},
}),
});

View File

@@ -13,6 +13,7 @@ import {
runWholeBlobP2a1HookSuite,
runWholeBlobP2bSuite,
} from "./wholeBlobSyncCarl.shared.test.mjs";
import { runWholeBlobCarlP2SecondClickSuite } from "./wholeBlobSyncCarlP2SecondClick.shared.test.mjs";
const { act, cleanup, renderHook } = await import("@testing-library/react");
const { relayClient } = await import("@/shared/api/relayClient");
@@ -233,3 +234,11 @@ runWholeBlobP2a1HookSuite({
);
},
});
runWholeBlobCarlP2SecondClickSuite({
label: "sort",
useHook: useChannelSortPreference,
makeEdit1: (r) => r.setSortModeFor("channels", "alpha"),
makeEdit2: (r) => r.setSortModeFor("groups", "recent"),
makeRemoteStore: () => ({ version: 1, groups: { remote: "recent" } }),
});

View File

@@ -0,0 +1,295 @@
// Carl P2 second-click regression (IMPORTANT/P2, Carl review 5096781519).
//
// Two timing variants — both must hold:
//
// Variant A (debounce window): A1 click pending (debounce timer running) →
// live H102 suppressed → A2 second click → debounce fires.
// Mutation (unconditional reseed / wasIdle || !inDebounceWindow):
// publishBaseline = H102 → A2 published over H102.
//
// Variant B (in-flight): A1's debounce fires → A1 preflight parked →
// live H102 suppressed → A2 second click → A1 preflight released (stale) →
// A2 cycle runs. Mutation (wasIdle || !inDebounceWindow, timer already null):
// publishBaseline = H102 → A2 published over H102.
//
// With correct fix (wasIdle || lastIsOwnAttempt — seed only for new sequence
// or own in-flight attempt; preserve for suppressed live remote):
// Both variants: H102 ∉ ambiguousAttemptIds → lastIsOwnAttempt=false →
// publishBaseline preserved as {0,""} → remoteAdvancedSince = true
// → ADOPT H102. H102's remote-only changes are preserved.
import assert from "node:assert/strict";
import test, { mock } from "node:test";
import { relayClient } from "@/shared/api/relayClient";
import {
makeHookTimerBed,
installEchoTauri,
} from "./sidebarSyncTestHelpers.mjs";
/**
* Hook-layer Carl P2 second-click regression for a single whole-blob lane.
*
* @param {object} opts
* @param {string} opts.label "sections"|"sort"
* @param {Function} opts.useHook the hook under test
* @param {Function} opts.makeEdit1 (hookResult) => void — first click
* @param {Function} opts.makeEdit2 (hookResult) => void — second click
* @param {Function} opts.makeRemoteStore () => store (live remote H102 content)
*/
export function runWholeBlobCarlP2SecondClickSuite({
label,
useHook,
makeEdit1,
makeEdit2,
makeRemoteStore,
}) {
// --- Variant A: second click while debounce timer is still running -----------
test(`Carl-P2 ${label} [debounce-window]: second click must not reseed baseline from suppressed live head — H102 must remain a genuine advance (adopt)`, async () => {
const { act, cleanup, renderHook } = await import("@testing-library/react");
const pubkey = `pk-p2-sc-dw-${label}`;
const relayUrl = `wss://r.p2sc.dw.${label}`;
const tauri = installEchoTauri(pubkey);
const H102 = tauri.mintHead(
makeRemoteStore(),
200,
`evt-h102-p2sc-dw-${label}`,
);
H102.pubkey = pubkey;
H102.kind = 30078;
H102.tags = [
["d", label === "sections" ? "channel-sections" : "channel-sort"],
];
const live = { cb: null };
const origFetch = relayClient.fetchEvents;
const origSubscribeLive = relayClient.subscribeLive;
const origPublish = relayClient.publishEvent;
const origReconnects = relayClient.subscribeToReconnects;
const publishCalls = [];
let fetchCalls = 0;
relayClient.fetchEvents = async () => {
fetchCalls++;
if (fetchCalls === 1) return []; // bootstrap: no remote head
return [H102]; // pre-publish: H102 is now the relay head
};
relayClient.subscribeLive = async (_f, cb) => {
live.cb = cb;
return async () => {};
};
relayClient.publishEvent = async (evt) => {
publishCalls.push(evt);
};
relayClient.subscribeToReconnects = () => () => {};
const { fireDelay, restore: restoreTimers } = makeHookTimerBed();
let hook = null;
try {
await act(async () => {
hook = renderHook(() => useHook(pubkey, relayUrl));
for (let i = 0; i < 80; i++) await Promise.resolve();
});
// A1: first click — debounce timer scheduled at 2000ms.
await act(async () => {
makeEdit1(hook.result.current);
for (let i = 0; i < 20; i++) await Promise.resolve();
});
// H102 arrives live while debounce is running — suppressed by hasPendingEdit.
assert.ok(
live.cb !== null,
`${label}: subscribeLive callback must be registered`,
);
await act(async () => {
live.cb(H102);
for (let i = 0; i < 20; i++) await Promise.resolve();
});
// A2: second click — debounce timer still running (pendingStore !== null).
// Fix: wasIdle=false, H102 ∉ ambiguousAttemptIds → lastIsOwnAttempt=false
// → baseline preserved {0,""}. Mutation catches this via Variant B.
await act(async () => {
makeEdit2(hook.result.current);
for (let i = 0; i < 20; i++) await Promise.resolve();
});
// Fire debounce. Pre-publish fetch returns H102.
// Fix: baseline={0,""} → remoteAdvancedSince → ADOPT. publishCalls===0.
await fireDelay(2000);
for (let i = 0; i < 100; i++) await Promise.resolve();
assert.equal(
publishCalls.length,
0,
`Carl-P2 ${label} [debounce-window]: H102 must be adopted — ` +
`reseeding baseline from suppressed live head silently overwrites H102`,
);
hook.unmount();
} finally {
cleanup();
tauri.restore();
relayClient.fetchEvents = origFetch;
relayClient.subscribeLive = origSubscribeLive;
relayClient.publishEvent = origPublish;
relayClient.subscribeToReconnects = origReconnects;
restoreTimers();
window.localStorage.clear();
mock.reset();
}
});
// --- Variant B: second click while A1's publish is in flight -----------------
// A1's debounce has fired (debounceTimer=null, publishInFlight=true).
// H102 arrives suppressed. A2 click: wasIdle=false, H102 ∉ ambiguousAttemptIds
// → lastIsOwnAttempt=false. Old predicate (wasIdle || !inDebounceWindow) reseeds
// baseline to H102 → A2 publishes over H102.
// Correct predicate (wasIdle || lastIsOwnAttempt): preserves → adopts H102.
test(`Carl-P2 ${label} [in-flight]: second click after debounce fired must not reseed baseline from suppressed live head — H102 must remain a genuine advance (adopt)`, async () => {
const { act, cleanup, renderHook } = await import("@testing-library/react");
const pubkey = `pk-p2-sc-if-${label}`;
const relayUrl = `wss://r.p2sc.if.${label}`;
const tauri = installEchoTauri(pubkey);
const H102 = tauri.mintHead(
makeRemoteStore(),
200,
`evt-h102-p2sc-if-${label}`,
);
H102.pubkey = pubkey;
H102.kind = 30078;
H102.tags = [
["d", label === "sections" ? "channel-sections" : "channel-sort"],
];
const live = { cb: null };
const origFetch = relayClient.fetchEvents;
const origSubscribeLive = relayClient.subscribeLive;
const origPublish = relayClient.publishEvent;
const origReconnects = relayClient.subscribeToReconnects;
const publishCalls = [];
let fetchCalls = 0;
// A1-preflight resolver: lets us park A1's fetch until after A2 clicks.
let releaseA1Preflight = null;
relayClient.fetchEvents = async () => {
fetchCalls++;
if (fetchCalls === 1) return []; // bootstrap
if (fetchCalls === 2) {
// A1 preflight — park until we've delivered H102 and clicked A2.
await new Promise((resolve) => {
releaseA1Preflight = resolve;
});
return []; // returns empty so A1 would publish — but gen CAS exits it
}
// fetchCalls >= 3: A2 preflight — H102 is now the relay head.
return [H102];
};
relayClient.subscribeLive = async (_f, cb) => {
live.cb = cb;
return async () => {};
};
relayClient.publishEvent = async (evt) => {
publishCalls.push(evt);
};
relayClient.subscribeToReconnects = () => () => {};
const { fireDelay, restore: restoreTimers } = makeHookTimerBed();
let hook = null;
try {
// Mount + bootstrap.
await act(async () => {
hook = renderHook(() => useHook(pubkey, relayUrl));
for (let i = 0; i < 80; i++) await Promise.resolve();
});
// A1 click — publishBaseline frozen to {0,""} (wasIdle=true).
await act(async () => {
makeEdit1(hook.result.current);
for (let i = 0; i < 20; i++) await Promise.resolve();
});
// Fire A1's debounce: debounceTimer→null, startCycle()→doPublish(A1,1)
// → fetchOwnBlobBeforePublish → fetchEvents call #2 → PARKED.
// publishInFlight=true, pendingStore=A1, pendingGeneration=1.
await act(async () => {
// fireDelay fires the timer fn and drains microtasks; the async
// doPublish chain starts but parks at the awaited fetchEvents.
await fireDelay(2000);
for (let i = 0; i < 30; i++) await Promise.resolve();
});
// H102 arrives while A1's preflight is parked — suppressed by hasPendingEdit.
assert.ok(
live.cb !== null,
`${label}: subscribeLive callback must be registered`,
);
await act(async () => {
live.cb(H102);
for (let i = 0; i < 20; i++) await Promise.resolve();
});
// A2 click — pendingStore !== null (A1 still pending), debounceTimer=null
// (timer already fired). Fix: wasIdle=false, H102 ∉ ambiguousAttemptIds
// → lastIsOwnAttempt=false → baseline preserved {0,""}.
// Mutation: wasIdle || !inDebounceWindow → inDebounceWindow=false → reseeds to H102.
await act(async () => {
makeEdit2(hook.result.current);
for (let i = 0; i < 20; i++) await Promise.resolve();
});
// Release A1's parked preflight → returns [] → A1's doPublish checks
// gen(1) !== pendingGeneration(2) → exits stale without publishing.
// startCycle() fires from finally → A2's cycle starts → schedules
// a fresh 2000ms debounce for A2's pending edit.
await act(async () => {
assert.ok(
releaseA1Preflight !== null,
`${label}: A1 preflight must have been parked`,
);
releaseA1Preflight();
for (let i = 0; i < 60; i++) await Promise.resolve();
});
// Fire A2's debounce timer so doPublish(A2) runs → fetchEvents #3 →
// remoteAdvancedSince arbitration. Without this the timer sits non-null
// and publishCalls===0 passes vacuously under both predicates.
await fireDelay(2000);
for (let i = 0; i < 100; i++) await Promise.resolve();
// A2's cycle: pre-publish fetch returns H102 (fetchCalls=3).
// Fix: baseline={0,""} → remoteAdvancedSince(H102,{0,""})=true → ADOPT.
// Broken: baseline=H102 → equality → publishEvent called → test fails.
for (let i = 0; i < 100; i++) await Promise.resolve();
assert.equal(
publishCalls.length,
0,
`Carl-P2 ${label} [in-flight]: second click after debounce fired must not reseed baseline — ` +
`wasIdle||!inDebounceWindow reseeds H102 as baseline → A2 publishes over H102`,
);
hook.unmount();
} finally {
cleanup();
tauri.restore();
relayClient.fetchEvents = origFetch;
relayClient.subscribeLive = origSubscribeLive;
relayClient.publishEvent = origPublish;
relayClient.subscribeToReconnects = origReconnects;
restoreTimers();
window.localStorage.clear();
mock.reset();
}
});
}

View File

@@ -188,10 +188,8 @@ export class WholeBlobSyncManager<S> {
// 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.
// publish() defers the debounce timer until bootstrap resolves; when bootstrap
// is never called, both flags stay false and the timer schedules immediately.
//
// The P2a queue-until-bootstrap contract: no publish debounce fires between
// bootstrap starting and bootstrap resolving, so a click during an unresolved
@@ -203,8 +201,7 @@ export class WholeBlobSyncManager<S> {
// 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 the bootstrap-result head, so the baseline is not {0,""}
// at doPublish and this flag is never consulted.)
// publishBaseline to the bootstrap-result head, so this flag is never consulted.)
private bootstrapFailed = false;
// Disarmed once any head is independently observed after a failed bootstrap
// (live relay subscription or a reconnect/periodic fetch). When disarmed, the
@@ -500,14 +497,20 @@ export class WholeBlobSyncManager<S> {
isRestoredReplay = false,
restoredQueuedAt?: number,
): boolean {
const wasIdle = this.pendingStore === null;
// Reseed only when (a) starting a new sequence (wasIdle) or (b)
// lastRemoteHead is our OWN attempt (lastIsOwnAttempt: eventId ∈
// ambiguousAttemptIds). Case (b) lets foldSupersedingAttemptWinner
// repair the baseline on a same-second collision. All other superseding
// edits preserve the existing baseline: lastRemoteHead may carry a
// suppressed live H102 that must not be silently overwritten (Carl P2).
const lastIsOwnAttempt =
this.lastRemoteHead.eventId !== "" &&
this.ambiguousAttemptIds.has(this.lastRemoteHead.eventId);
this.pendingStore = store;
++this.pendingGeneration;
// Freeze the canonical head this edit races against at queue time. The
// pre-publish check compares against this baseline — not the mutable
// watermark — so a live event during the debounce window that advances the
// watermark still gets adopted (baseline only advances via our own
// confirmed publishes). For a restored replay: use canonicalMax(current,
// bootstrapResultHead) from the immutable bootstrap snapshot (C2):
// Freeze the canonical head this edit races against at queue time.
// For a restored replay: use canonicalMax(current, bootstrapResultHead):
// No prior pending: publishBaseline={0,""}, canonicalMax sets bootstrap head.
// Blocked-bootstrap: releaseDeferred already set it; re-apply is idempotent.
// lastRemoteHead excluded (may carry suppressed H102 from subscribeLive).
@@ -516,9 +519,10 @@ export class WholeBlobSyncManager<S> {
this.publishBaseline,
this.bootstrapResultHead,
);
} else {
} else if (wasIdle || lastIsOwnAttempt) {
this.publishBaseline = { ...this.lastRemoteHead };
}
// else: superseding edit — preserve baseline (Carl P2).
// Suppress failed-bootstrap exception for replays (P1/C1); preserve original
// queuedAt for the failed-bootstrap adopt-guard (publish when relay head
// createdAt <= restoredQueuedAt — restored edit is genuinely newer).