Skip to content

Commit 350d4d3

Browse files
committed
rpc: fix sync-driver build and same-rev pull divergence
The fetch watermark became a rev/path cursor, but reconcileWatermarks still read remoteWatermarks.fetchRev, a field that no longer exists on the watermarks() result. The reference broke the type check, failed the rpc build, and cascaded into every workspace that depends on the built package. Read the rev off the cursor instead. The same cursor conversion also widened the pullOnce recovery predicate. The inline reset path exists to absorb a remote that forgot what we pushed, which shows up as the remote's applied push cursor rev regressing below our local pushRev. Comparing the full cursor instead of its rev folded a second, unrelated case into the recoverable set: a same-rev partial cursor, where the remote applied part of the rev we pushed and then tore down mid-apply. That is a genuine cross-side invariant violation. Resetting and replaying cannot mend lost state inside a rev the remote claimed to hold, so the predicate is rev-only again and the assertion surfaces the partial-apply case. Point the envelope-disposal test at a same-rev partial cursor so it exercises a real invariant trip rather than the recoverable rev regression, which the inline reset now swallows by design.
1 parent 1127ae2 commit 350d4d3

2 files changed

Lines changed: 31 additions & 18 deletions

File tree

packages/rpc/src/sync-driver.test.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import {
2+
type ChangeCursor,
23
type ChangeEntry,
34
currentRev,
45
Database,
@@ -58,7 +59,7 @@ async function drainStream<T>(stream: ReadableStream<T>): Promise<T[]> {
5859
// applyChanges to throw on a missing object mid-stream).
5960
function trackFetchDisposal(
6061
rpc: SyncRPC,
61-
opts: { lowerCursor?: boolean; failHasObjects?: boolean } = {},
62+
opts: { appliedPushCursor?: ChangeCursor; failHasObjects?: boolean } = {},
6263
): { rpc: SyncRPC; disposeCount: () => number } {
6364
let disposeCount = 0;
6465
const wrapped = new Proxy(rpc as object, {
@@ -68,7 +69,9 @@ function trackFetchDisposal(
6869
const real = await Reflect.get(target, prop, receiver).call(target, ...args);
6970
return {
7071
...real,
71-
...(opts.lowerCursor ? { appliedPushCursor: { rev: 0, path: null } } : {}),
72+
...(opts.appliedPushCursor !== undefined
73+
? { appliedPushCursor: opts.appliedPushCursor }
74+
: {}),
7275
[Symbol.dispose]() {
7376
disposeCount += 1;
7477
},
@@ -460,13 +463,17 @@ describe("sync driver — pullOnce envelope disposal", () => {
460463
const local = new Database(new SQLiteTestStorage());
461464
initializeSchema(local, () => 1000);
462465
// Local claims to have pushed rev 42; the wrapper echoes back a
463-
// rev-0 cursor, so assertAppliedPushCursor throws before the
464-
// stream is ever read.
466+
// same-rev partial cursor (rev 42 but stalled mid-apply at a
467+
// path). That is a non-recoverable divergence — the inline
468+
// reset path only catches a rev regression — so
469+
// assertAppliedPushCursor throws before the stream is read.
465470
writeWatermark(local, "pushRev", 42);
466471
const providerR = new SQLiteWorkspaceProvider(remote.db, { now: () => 1 });
467472
providerR.writeFileSync("/seed.txt", "x");
468473

469-
const tracked = trackFetchDisposal(remote.rpc, { lowerCursor: true });
474+
const tracked = trackFetchDisposal(remote.rpc, {
475+
appliedPushCursor: { rev: 42, path: "/partial.txt" },
476+
});
470477
await expect(pullOnce(local, tracked.rpc)).rejects.toThrow(/cross-side invariant violated/i);
471478
expect(tracked.disposeCount()).toBe(1);
472479
} finally {

packages/rpc/src/sync-driver.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -107,10 +107,10 @@ async function pullOnceImpl(
107107
try {
108108
const { currentCursor, appliedPushCursor } = fetchResult;
109109
// Cross-side watermark divergence. Two shapes are recoverable:
110-
// * appliedPushCursor < {rev: localPushRev, path: null}: the
111-
// remote forgot what we pushed (typically a process-lifetime
112-
// wsd restart while the WebSocket survived, so
113-
// reconcileWatermarks on connect never re-ran).
110+
// * appliedPushCursor.rev < localPushRev: the remote forgot
111+
// what we pushed (typically a process-lifetime wsd restart
112+
// while the WebSocket survived, so reconcileWatermarks on
113+
// connect never re-ran).
114114
// * currentCursor < after: the remote's log is shorter than we
115115
// remember — same root cause, different symptom.
116116
// Both are the inline equivalent of reconcileWatermarks: reset
@@ -119,10 +119,16 @@ async function pullOnceImpl(
119119
// re-ships everything incrementally and the receiver's
120120
// alreadyApplied() check absorbs the redundant work.
121121
//
122+
// The divergence test is rev-only on purpose. A same-rev partial
123+
// appliedPushCursor (the remote applied part of the rev we
124+
// pushed, then tore down mid-apply) is not a recoverable race —
125+
// it means the receiver lost state inside a rev it told us it
126+
// had. Resetting and replaying cannot mend that, so we let the
127+
// assertion below surface it instead of looping.
128+
//
122129
// A second divergence after a reset is a real protocol break:
123130
// surface it via the assertion below rather than loop.
124-
const pushDiverged =
125-
compareChangeCursors(appliedPushCursor, { rev: localPushRev, path: null }) < 0;
131+
const pushDiverged = appliedPushCursor.rev < localPushRev;
126132
const fetchDiverged = compareChangeCursors(currentCursor, after) < 0;
127133
if (!retried && (pushDiverged || fetchDiverged)) {
128134
// Cancel the stream before disposing the envelope. For a real
@@ -410,20 +416,20 @@ export async function reconcileWatermarks(
410416
fetchRevReset = true;
411417
}
412418

413-
// The remote's fetchRev is the largest senderRev it has applied
414-
// from us — every push handler advances fetchRev to the incoming
415-
// senderRev, and fetchChanges echoes that value back as
416-
// appliedPushRev. If it's below our local pushRev, the remote has
417-
// not seen what we claimed to ship; reset our pushRev so the next
418-
// pushOnce re-baselines from rev 0.
419+
// The remote's fetch cursor rev is the largest senderRev it has
420+
// applied from us — every push handler advances the fetch cursor to
421+
// the incoming senderRev, and fetchChanges echoes that cursor back
422+
// as appliedPushCursor. If its rev is below our local pushRev, the
423+
// remote has not seen what we claimed to ship; reset our pushRev so
424+
// the next pushOnce re-baselines from rev 0.
419425
//
420426
// We deliberately do NOT compare against remoteWatermarks.pushRev:
421427
// that field is the remote's own *outbound* push progress and
422428
// stays at 0 in topologies where the remote never initiates a push
423429
// (e.g. the container side of a DO↔container backend), which would
424430
// make every reconcile spuriously reset pushRev and force a full
425431
// re-push on every reconnect.
426-
if (remoteWatermarks.fetchRev < localPushRev) {
432+
if (remoteWatermarks.fetchCursor.rev < localPushRev) {
427433
writeWatermark(db, "pushRev", 0, backend);
428434
pushRevReset = true;
429435
}

0 commit comments

Comments
 (0)