fix(sharing): stop a switch mistaking its own reconcile for a foreign one

The scope mirror is watched so a switch this browser did not begin still
holds every shell until /me reconciles it: another tab on an older
bundle, the referential action behind an account deletion, an operator.
It recognised its own by comparing the mirrored scope against the scope
it was switching to, and that is not a true test. fetchMe mirrors
whatever record the server currently reports and it runs DURING a switch,
both when postSwitchWithReconcile reconciles a lost compare-and-set and
when a second tab reloads beside the initiator. At that moment the server
still reports the record being left, so the write did not match and was
read as somebody else's switch.

What followed was silent. A foreign write publishes a new transition id,
and every terminal call opens by checking the id it was handed, so the
commit that followed was dropped without a trace and the expected scope
was left naming a record no /me would ever confirm. Nothing could release
the hold, and both tabs sat behind the hydration gate until the stored
transition aged out thirty seconds later.

That is the intermittent browser failure that has been carried through
three rounds as a canary and never explained: a peer reload racing a
switch, a gate that never lifts, and a run forty-six seconds long made of
a fifteen-second wait and a thirty-second one. On the selection that
provokes it, eight runs failed three times before and twenty failed none
after, and the runs that used to take fifty-seven seconds now take
twenty-five.

A write is foreign only when nothing of this browser's own is in flight.
Ignoring it while held costs nothing, because a held browser is already
in the state that branch would install. The rule is a named function so
it can be checked in a suite that has no DOM, and the property the guard
exists for is pinned beside it: a response that disagrees with a
committed scope is stale and still must not release.
This commit is contained in:
Marc-André Bombeck committed 2026-08-07 12:12:39 +02:00
1 parent ad094cba4d
commit 65531c359a
2 files changed
+131 -8

No files matched your search

@@ -6,6 +6,7 @@ import {
commitRecordSessionTransition,
getRecordSessionTransition,
resolveUnknownRecordSessionTransition,
isForeignScopeWrite,
settleRefusedRecordSessionTransition,
settleRecordSessionTransition,
} from "@/lib/query-keys/record-session-transition";
@@ -93,4 +94,95 @@ describe("record session transition", () => {
expectedScope: undefined,
});
});
// ── The scope mirror is not a foreign switch ─────────────────────────────
//
// The listener that watches the scope mirror catches a switch this browser
// did not begin. It recognised its OWN by comparing the mirrored scope
// against `expectedScope`, which is not a true test: `fetchMe()` mirrors
// whatever record the server currently reports and it runs during a switch —
// `postSwitchWithReconcile` calls it to reconcile a lost compare-and-set, and
// a peer tab reloading beside the initiator calls it too. At that moment the
// server still reports the record being left, so the write did not match and
// was read as foreign.
//
// The consequence is silent and total. A foreign write publishes a NEW id,
// and every terminal call opens with `if (transition.id !== id) return`, so
// the commit that follows is dropped without a trace and `expectedScope` is
// left naming a record no `/me` will confirm. Both tabs then sit behind the
// hydration gate until the stored transition ages out thirty seconds later.
//
// The rule is extracted rather than driven through a StorageEvent because
// this suite runs in `node`: with no `window` the listener never attaches,
// so a DOM-shaped test here would pass without exercising anything.
it("treats a scope write as foreign only when nothing of ours is in flight", () => {
expect(
isForeignScopeWrite({
id: null,
phase: "ready",
expectedScope: undefined,
}),
).toBe(true);
});
it("does not treat its own in-flight switch as a foreign write", () => {
// Both non-ready phases, each with an `expectedScope` that does NOT match
// what a reconcile mirrors — the exact combination the old comparison got
// wrong.
expect(
isForeignScopeWrite({
id: "t1",
phase: "blocking",
expectedScope: "shared-record",
}),
).toBe(false);
expect(
isForeignScopeWrite({
id: "t1",
phase: "resolving",
expectedScope: "shared-record",
}),
).toBe(false);
});
it("does not treat an outcome-unknown hold as a foreign write either", () => {
// `resolveUnknownRecordSessionTransition` leaves `expectedScope`
// undefined, which never matched a mirrored scope, so under the old
// comparison every scope write during that hold reassigned the id.
expect(
isForeignScopeWrite({
id: "t1",
phase: "resolving",
expectedScope: undefined,
}),
).toBe(false);
});
it("keeps a commit landing on the transition it was issued for", () => {
// The end-to-end shape without the DOM: the id must not move between
// begin and commit, or the commit is dropped and no `/me` releases.
const transitionId = beginRecordSessionTransition("shared-record");
expect(getRecordSessionTransition().id).toBe(transitionId);
commitRecordSessionTransition(transitionId, "shared-record");
expect(getRecordSessionTransition()).toMatchObject({
phase: "resolving",
expectedScope: "shared-record",
});
settleRecordSessionTransition("shared-record");
expect(getRecordSessionTransition().phase).toBe("ready");
});
it("still refuses to release on a scope the transition did not expect", () => {
// The property the guard exists for, pinned so the change above cannot be
// read as permission to release on anything: a `/me` that disagrees with a
// COMMITTED scope is stale and must not release.
const transitionId = beginRecordSessionTransition("shared-record");
commitRecordSessionTransition(transitionId, "shared-record");
settleRecordSessionTransition(null);
expect(getRecordSessionTransition().phase).toBe("resolving");
});
});
@@ -156,14 +156,11 @@ function ensureTransport(): void {
event.newValue === null || event.newValue.length === 0
? null
: event.newValue;
// A current protocol transition already names this scope. The scope
// mirror is its second storage write, not a separate legacy switch.
if (
transition.phase !== "ready" &&
transition.expectedScope === expectedScope
) {
return;
}
// A transition of this browser's own is already in flight and owns the
// outcome — including the scope its own reconcile mirrors. See
// `isForeignScopeWrite` for what treating that as a foreign switch
// cost.
if (!isForeignScopeWrite(transition)) return;
publish({
id: makeId(),
phase: "resolving",
@@ -185,6 +182,40 @@ function ensureTransport(): void {
}
}
/**
* Whether a write to the scope mirror is a switch this browser did not begin.
*
* The mirror is watched so that a switch from outside this protocol — another
* tab on an older bundle, the referential action behind an account deletion,
* an operator — still holds every shell until `/me` reconciles it. It used to
* recognise its own by comparing the mirrored scope against `expectedScope`,
* and that is too narrow to be true.
*
* `fetchMe()` mirrors whatever record the server currently reports, and it runs
* DURING a switch: `postSwitchWithReconcile` calls it to reconcile a lost
* compare-and-set, and a peer tab reloading beside the initiator calls it too.
* At that moment the server still reports the record being switched away from,
* so the mirrored scope is not the expected one and the write read as foreign.
*
* What followed was silent. The branch published a NEW transition id, and every
* terminal call is keyed on the id its caller was handed —
* `commitRecordSessionTransition` and `resolveUnknownRecordSessionTransition`
* both open with `if (transition.id !== id) return`. So the commit was dropped
* without a trace, `expectedScope` was left naming a record no `/me` would ever
* confirm, and `settleRecordSessionTransition` refused every answer the server
* gave. Both tabs sat behind the hydration gate until the stored transition
* aged past `TRANSITION_MAX_AGE_MS` — thirty seconds, which is where the
* intermittent forty-six-second failures in the browser suite came from.
*
* So a write is foreign only when nothing of this browser's own is in flight.
* Ignoring it while held costs nothing: `resolving` already means "held,
* outcome unknown, `/me` decides", which is precisely the state this branch
* would otherwise install.
*/
export function isForeignScopeWrite(current: RecordSessionTransition): boolean {
return current.phase === "ready";
}
function seed(): void {
if (seeded) return;
if (typeof window === "undefined") {