feedback7 G-TOOLS: a reconnect token cannot be claimed twice by two tabs started apart (PR #299 by @siruimei07)

Applied as-is. A tab that is still choosing a reconnect token now answers another tab's query for the same candidate hashes, so a tab started during the 150 ms window cannot adopt a token the earlier tab is also considering. The smaller tab id still wins, and a tab that already holds the token keeps it.

Tests: node --test test/client-identity-session.test.js test/client-static.test.js — 319 pass, 0 fail.

public/js/net.js

@@ -726,7 +726,7 @@ export function createIdentity(deps = {}) {
   let initPromise = null;
   let initialized = false;
   let current = null;    // token this tab uses (null = let the server create a session)
-  let resolving = null;  // { taken: Set<hash> } while init() waits for claims
+  let resolving = null;  // { hashes: Set<hash>, taken: Set<hash> } while init() waits for claims
 
   const readRecent = () => {
     try {
@@ -751,7 +751,12 @@ export function createIdentity(deps = {}) {
         if (m.hashes.includes(h)) post({ type: 'mine', hash: h, to: m.from });
       }
       // Another tab is choosing at the same time: the smaller tab id has precedence.
-      if (resolving && m.from < tabId) for (const h of m.hashes) if (typeof h === 'string') resolving.taken.add(h);
+      // Answer for pending candidates too: a later channel may have missed our initial query.
+      if (resolving) for (const h of m.hashes) {
+        if (typeof h !== 'string') continue;
+        if (m.from < tabId) resolving.taken.add(h);
+        else if (resolving.hashes.has(h)) post({ type: 'mine', hash: h, to: m.from });
+      }
     } else if (m.type === 'mine' && resolving && m.to === tabId && typeof m.hash === 'string') {
       resolving.taken.add(m.hash);
     }
@@ -768,8 +773,8 @@ export function createIdentity(deps = {}) {
     // Without a channel we cannot tell whether a shared token is in use: never adopt one.
     const candidates = [...new Set([ownOk, ...(channel ? readRecent() : [])].filter(isToken))];
     if (!channel || candidates.length === 0) return ownOk;
-    resolving = { taken: new Set() };
-    post({ type: 'who', hashes: candidates.map(tokenHash) });
+    resolving = { hashes: new Set(candidates.map(tokenHash)), taken: new Set() };
+    post({ type: 'who', hashes: [...resolving.hashes] });
     await new Promise((r) => setTimer(r, queryMs));
     const { taken } = resolving;
     resolving = null;

test/client-identity-session.test.js

@@ -0,0 +1,143 @@
+// Real BroadcastChannel election followed by real client/server session handshakes.
+import { test } from 'node:test';
+import assert from 'node:assert/strict';
+import { once } from 'node:events';
+import { BroadcastChannel } from 'node:worker_threads';
+import WebSocket from 'ws';
+import { startServer } from '../server/index.js';
+import { CLAIM_QUERY_MS, createIdentity, Net } from '../public/js/net.js';
+
+function storage(seed = {}) {
+  const values = new Map(Object.entries(seed));
+  return {
+    getItem: (key) => values.get(key) ?? null,
+    setItem: (key, value) => values.set(key, String(value)),
+    removeItem: (key) => values.delete(key),
+  };
+}
+
+function nextEvent(net, type) {
+  return new Promise((resolve, reject) => {
+    const off = net.on(type, (value) => { clearTimeout(timer); off(); resolve(value); });
+    const timer = setTimeout(() => { off(); reject(new Error(`Timed out waiting for ${type}`)); }, 2000);
+  });
+}
+
+async function harness(t) {
+  const srv = await startServer({ port: 0, host: '127.0.0.1', quiet: true });
+  const clients = [], channels = [], replaced = [];
+  const local = storage();
+  const channelName = `sp.identity.test.${process.pid}.${srv.port}`;
+  t.after(async () => {
+    clients.forEach((net) => net.close());
+    channels.forEach((channel) => channel.close());
+    await srv.close();
+  });
+  return {
+    local, replaced,
+    tab(tabId, session = storage(), setTimeout) {
+      const channel = new BroadcastChannel(channelName);
+      channels.push(channel);
+      const id = createIdentity({ local, session, channel, tabId, setTimeout });
+      return { id, session, channel };
+    },
+    async connect(name, tab) {
+      const net = new Net({ url: `ws://127.0.0.1:${srv.port}/ws`, WebSocket, getToken: () => tab?.id.getToken() ?? null });
+      clients.push(net);
+      net.on('replaced', (error) => replaced.push({ name, code: error.code }));
+      net.on('welcome', (welcome) => tab?.id.saveToken(welcome.token));
+      const welcome = nextEvent(net, 'welcome');
+      net.setName(name);
+      return { net, welcome: await welcome };
+    },
+  };
+}
+
+async function closeClient(net) {
+  const closed = once(net.ws, 'close');
+  net.close();
+  await closed;
+}
+
+async function assertOnline(h, clients) {
+  // A round trip from each socket also lets a queued replacement close reach the client.
+  const replies = await Promise.allSettled(clients.map(({ net }) => net.request('ping', { c: Date.now() })));
+  assert.deepEqual(h.replaced, [], 'no client is replaced by another tab');
+  for (const [index, client] of clients.entries()) {
+    assert.equal(client.net.status, 'online');
+    assert.equal(client.net.lastError, null);
+    assert.equal(replies[index].status, 'fulfilled');
+    assert.equal(replies[index].value.t, 'pong');
+  }
+}
+
+for (const [firstId, secondId] of [['a-tab', 'b-tab'], ['b-tab', 'a-tab']]) {
+  test(`staggered tabs ${firstId} then ${secondId} keep separate server sessions`, { timeout: 5000 }, async (t) => {
+    const h = await harness(t);
+    const seed = await h.connect('Seed');
+    await closeClient(seed.net);
+    h.local.setItem('sp.tokens', JSON.stringify([seed.welcome.token]));
+
+    const deadlines = [];
+    const holdDeadline = (fn, ms) => { assert.equal(ms, CLAIM_QUERY_MS); deadlines.push(fn); };
+    const first = h.tab(firstId, storage(), holdDeadline);
+    const firstInit = first.id.init();
+    // The first who has already been sent when the second channel is created.
+    const second = h.tab(secondId, storage(), holdDeadline);
+    const delivered = new Promise((resolve) => {
+      second.channel.addEventListener('message', (event) => {
+        if (event.data.type === 'test.barrier') resolve();
+      });
+    });
+    first.channel.addEventListener('message', (event) => {
+      if (event.data.type === 'who') first.channel.postMessage({ type: 'test.barrier' });
+    });
+    const secondInit = second.id.init();
+    // FIFO delivery puts this barrier after any real reply to the later who. Both default
+    // 150 ms deadlines remain pending, without relying on sleeps or a fake channel hub.
+    await delivered;
+    assert.equal(deadlines.length, 2);
+    deadlines[0]();
+    await firstInit;
+    deadlines[1]();
+    await secondInit;
+
+    const a = await h.connect('First', first);
+    const b = await h.connect('Second', second);
+    await assertOnline(h, [a, b]);
+    assert.notEqual(a.welcome.token, b.welcome.token);
+    assert.notEqual(a.welcome.playerId, b.welcome.playerId);
+    const resumed = firstId < secondId ? a : b;
+    assert.equal(resumed.welcome.token, seed.welcome.token);
+    assert.equal(resumed.welcome.playerId, seed.welcome.playerId);
+  });
+}
+
+test('live token ownership survives a duplicated tab, reload and reconnect', { timeout: 5000 }, async (t) => {
+  const h = await harness(t);
+  const owner = h.tab('z-owner');
+  await owner.id.init();
+  const a = await h.connect('Owner', owner);
+
+  // A duplicated tab copies sessionStorage, but its smaller tab ID cannot evict a live owner.
+  const duplicate = h.tab('a-duplicate', storage({ 'sp.token': a.welcome.token }));
+  await duplicate.id.init();
+  const b = await h.connect('Duplicate', duplicate);
+  assert.notEqual(a.welcome.token, b.welcome.token);
+  assert.notEqual(a.welcome.playerId, b.welcome.playerId);
+  await assertOnline(h, [a, b]);
+
+  await closeClient(a.net);
+  owner.channel.close();
+  const reloaded = h.tab('z-reloaded', owner.session);
+  await reloaded.id.init();
+  const a2 = await h.connect('Owner', reloaded);
+  assert.equal(a2.welcome.token, a.welcome.token);
+  assert.equal(a2.welcome.playerId, a.welcome.playerId);
+  const reconnected = nextEvent(a2.net, 'welcome');
+  a2.net.reconnectNow();
+  const welcome = await reconnected;
+  assert.equal(welcome.token, a.welcome.token);
+  assert.equal(welcome.playerId, a.welcome.playerId);
+  await assertOnline(h, [a2, b]);
+});

test/client-static.test.js

@@ -790,6 +790,58 @@ describe('identity (reconnect-token selection across tabs)', () => {
     assert.equal(ty, null);
   });
 
+  for (const [firstId, secondId, expected] of [
+    ['a-tab', 'b-tab', ['tokShared', null]],
+    ['b-tab', 'a-tab', [null, 'tokShared']],
+  ]) {
+    test(`staggered channels: ${firstId} starts before ${secondId}`, async () => {
+      const { createIdentity, CLAIM_QUERY_MS } = await mod('net.js');
+      const hub = channelHub(), timers = fakeTimers(), local = memStorage();
+      local.setItem('sp.tokens', JSON.stringify(['tokShared']));
+      const firstSession = memStorage(), secondSession = memStorage();
+      const first = createIdentity({ local, session: firstSession, tabId: firstId,
+        channel: hub.create(), setTimeout: timers.setTimeout });
+      const firstPick = first.init();
+      timers.advance(30);
+      // This channel did not exist when the first tab broadcast its query.
+      const second = createIdentity({ local, session: secondSession, tabId: secondId,
+        channel: hub.create(), setTimeout: timers.setTimeout });
+      const secondPick = second.init();
+      await new Promise((r) => setImmediate(r));
+      timers.advance(CLAIM_QUERY_MS - 30);
+      assert.equal(await firstPick, expected[0]);
+      timers.advance(30);
+      assert.equal(await secondPick, expected[1]);
+      assert.deepEqual([first.getToken(), second.getToken()], expected);
+      assert.deepEqual([firstSession.getItem('sp.token'), secondSession.getItem('sp.token')], expected);
+    });
+  }
+
+  test('staggered elections reserve fallback candidates while a live owner keeps its token', async () => {
+    const { createIdentity, CLAIM_QUERY_MS } = await mod('net.js');
+    const hub = channelHub(), timers = fakeTimers(), local = memStorage();
+    const owner = createIdentity({ local, session: memStorage(), tabId: 'z-owner', channel: hub.create() });
+    await owner.init();
+    owner.saveToken('tokOwned');
+    local.setItem('sp.tokens', JSON.stringify(['tokOwned', 'tokFree']));
+    const first = createIdentity({ local, session: memStorage(), tabId: 'a-tab',
+      channel: hub.create(), setTimeout: timers.setTimeout });
+    const firstPick = first.init();
+    timers.advance(30);
+    const duplicateSession = memStorage();
+    duplicateSession.setItem('sp.token', 'tokOwned');
+    const second = createIdentity({ local, session: duplicateSession, tabId: 'b-tab',
+      channel: hub.create(), setTimeout: timers.setTimeout });
+    const secondPick = second.init();
+    await new Promise((r) => setImmediate(r));
+    timers.advance(CLAIM_QUERY_MS - 30);
+    assert.equal(await firstPick, 'tokFree');
+    timers.advance(30);
+    assert.equal(await secondPick, null);
+    assert.equal(duplicateSession.getItem('sp.token'), null, 'the copied live token is discarded');
+    assert.equal(owner.getToken(), 'tokOwned', 'a live owner outranks even a smaller tab id');
+  });
+
   test('without BroadcastChannel only the own token is used; init is idempotent; welcome during init wins', async () => {
     const { createIdentity } = await mod('net.js');
     const local = memStorage();