From c30bcd4531be83580b105af90dbd0f363ab489d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gerg=C5=91=20T=C3=B6rcsv=C3=A1ri?= Date: Tue, 7 Jul 2026 20:35:26 +0200 Subject: [PATCH] =?UTF-8?q?feat(collab):=20selection=20soft-locks=20?= =?UTF-8?q?=E2=80=94=20remote-selected=20items=20can't=20be=20dragged=20lo?= =?UTF-8?q?cally=20(collab-presence=200007)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While a peer has an item selected, local users can still select it for inspection but move/drag/rotate/delete skip it with an infobar naming the holder (native locked-item UX; enforced via the fork's PCBJAM_REMOTE_LOCK query — kicad 81f9cd80fd, the epic's first fork-touching phase). Overlapping holds (both grabbed inside the awareness propagation window) tie-break deterministically: lowest (user.id, clientID) keeps the item, every losing client auto-releases it. - lock-tiebreak.ts: pure policy — beats(), remoteLocks() (union of ALL other clients' selections incl. own user's other tabs, minus own-held-and-winning uuids so the winner isn't blocked mid-release), contestedReleases() - presence.ts: clients() (per-client view, no user dedupe) + self(); FIX for a pre-existing flaky stack overflow — resolveCollision re-entered itself synchronously via its own patch's awareness 'change' and could ping-pong on stale same-user states (~1-in-3 unit runs); re-entrancy guard defers re-resolution to the next genuine delivery - presence-kicad.ts: locks ride the kicadCollabSetRemote snapshot (`locks:[{uuid,name}]`); losing overlaps call kicadCollabReleaseSelection - wasm bindings (both TUs + merged dispatch): g_locks map + fork query install; kicadCollabReleaseSelection (cancelInteractive only when a tool stack is live — bare ESC would clear the whole selection — then selective RemoveItemFromSel + infobar + forced re-emit); kicadCollabTestGetLocked - tests: lock-tiebreak unit suite; presence-locks e2e for both editors (real move veto — pcbnew click+M hotkey since its default left-drag is rubber-band select, eeschema real drag — each with an unlocked control); two-tab tests/web/locks.spec.ts (lock propagation + deterministic tiebreak release + unlock on clear, passing vs real partykit) Spec: docs/features/collab-presence/0007 (closed repo). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01Lg5jwWuhFH5dL8hEcBDuP2 --- kicad | 2 +- tests/kicad/presence-locks-eeschema.spec.ts | 233 +++++++++++++ tests/kicad/presence-locks-pcbnew.spec.ts | 316 ++++++++++++++++++ tests/web/locks.spec.ts | 111 ++++++ wasm/bindings/eeschema_embind.cpp | 108 ++++++ wasm/bindings/kicad_editor_embind.cpp | 20 ++ wasm/bindings/pcbnew_embind.cpp | 116 +++++++ .../src/wasm/collab/cross-app.test.ts | 6 +- .../src/wasm/collab/lock-tiebreak.test.ts | 74 ++++ .../src/wasm/collab/lock-tiebreak.ts | 89 +++++ .../src/wasm/collab/presence-kicad.test.ts | 70 +++- .../src/wasm/collab/presence-kicad.ts | 27 ++ .../src/wasm/collab/presence.test.ts | 14 +- web/standalone/src/wasm/collab/presence.ts | 42 ++- 14 files changed, 1221 insertions(+), 7 deletions(-) create mode 100644 tests/kicad/presence-locks-eeschema.spec.ts create mode 100644 tests/kicad/presence-locks-pcbnew.spec.ts create mode 100644 tests/web/locks.spec.ts create mode 100644 web/standalone/src/wasm/collab/lock-tiebreak.test.ts create mode 100644 web/standalone/src/wasm/collab/lock-tiebreak.ts diff --git a/kicad b/kicad index 24c5854..81f9cd8 160000 --- a/kicad +++ b/kicad @@ -1 +1 @@ -Subproject commit 24c5854d5bf76fc908b8ba8a3bed63675d5067cf +Subproject commit 81f9cd80fdf9ce91c41776c8d3187ba7d8807ad6 diff --git a/tests/kicad/presence-locks-eeschema.spec.ts b/tests/kicad/presence-locks-eeschema.spec.ts new file mode 100644 index 0000000..f18f158 --- /dev/null +++ b/tests/kicad/presence-locks-eeschema.spec.ts @@ -0,0 +1,233 @@ +import type { Page } from "@playwright/test"; +import { test, expect } from "./fixtures"; + +/** + * eeschema selection soft-locks — C++ veto e2e (collab-presence 0007). + * Mirror of presence-locks-pcbnew.spec.ts: eeschema has NO native item-lock + * plumbing (the itemPassesFilter lock branch is #if 0), so the veto rides the + * pcbjam remote-lock check in narrowSelection's aCheckLocked path — a real + * drag gesture on a locked wire must not move it, the same gesture moves it + * unlocked, and ReleaseSelection strips exactly the contested uuid. + */ + +const WIRE1 = "22222222-0000-0000-0000-000000000001"; +const SAMPLE_SCH = `(kicad_sch +\t(version 20250114) +\t(generator "eeschema") +\t(generator_version "9.0") +\t(uuid "11111111-1111-1111-1111-111111111111") +\t(paper "A4") +\t(lib_symbols) +\t(wire (pts (xy 50.8 50.8) (xy 101.6 50.8)) (stroke (width 0) (type default)) (uuid "${WIRE1}")) +\t(wire (pts (xy 50.8 76.2) (xy 101.6 76.2)) (stroke (width 0) (type default)) (uuid "22222222-0000-0000-0000-000000000002")) +\t(sheet_instances (path "/" (page "1"))) +) +`; + +type FS = { mkdirTree(p: string): void; writeFile(p: string, d: string): void }; +type Mod = { + kicadOpenFile(p: string): unknown; + kicadCollabPresenceStart(): void; + kicadCollabSetRemote(j: string): void; + kicadCollabGetViewport(): string; + kicadCollabGetSelection(): string; + kicadCollabGetPos(id: string): string; + kicadCollabReleaseSelection(uuidsJson: string, holder: string): void; + kicadCollabTestGetLocked(): string; + kicadCollabTestSelectFirst(): string; +}; +type LocksWindow = { + FS: FS; + Module: Mod; + kicadCollab?: Record; +}; + +function hasAbort(l: { consoleLogs: string[]; errors: string[] }): boolean { + return [...l.consoleLogs, ...l.errors].some((s) => s.includes("Aborted(")); +} + +async function bootAndOpen(page: Page): Promise { + await page.goto("/kicad/eeschema.html"); + await expect(page.locator("#canvas")).toBeVisible({ timeout: 90000 }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: 90000 }); + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Partial }).Module; + return ( + typeof m?.kicadOpenFile === "function" && + typeof m?.kicadCollabTestGetLocked === "function" + ); + }, + null, + { timeout: 90000 }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || "").endsWith("Frame")), + null, + { timeout: 90000 }, + ); + + await page.evaluate( + ({ content }) => { + const w = window as unknown as LocksWindow; + const dir = "/home/kicad/documents"; + try { + w.FS.mkdirTree(dir); + } catch { + /* exists */ + } + const p = `${dir}/locks.kicad_sch`; + w.FS.writeFile(p, content); + w.Module.kicadOpenFile(p); + }, + { content: SAMPLE_SCH }, + ); + + await expect + .poll(() => page.title(), { timeout: 60000, intervals: [500] }) + .toMatch(/locks/i); + + await page.evaluate(() => { + (window as unknown as LocksWindow).Module.kicadCollabPresenceStart(); + }); +} + +function setLock(page: Page, locked: boolean): Promise { + return page.evaluate( + ({ id, locked: isLocked }) => { + const w = window as unknown as LocksWindow; + w.Module.kicadCollabSetRemote( + JSON.stringify({ + peers: [], + locks: isLocked ? [{ uuid: id, name: "bob" }] : [], + }), + ); + }, + { id: WIRE1, locked }, + ); +} + +const wirePos = (page: Page) => + page.evaluate(() => + (window as unknown as LocksWindow).Module.kicadCollabGetPos( + "22222222-0000-0000-0000-000000000001", + ), + ); + +/** Screen position of the wire's stored (start) point via the GAL transform. */ +async function wireScreenPos(page: Page): Promise<{ x: number; y: number }> { + const glId = await page.evaluate(() => { + const visible = Array.from(document.querySelectorAll('[id^="glcanvas-"]')) + .map((c) => c as HTMLCanvasElement) + .find( + (c) => + window.getComputedStyle(c).display !== "none" && + c.getBoundingClientRect().width > 0, + ); + return visible?.id ?? null; + }); + expect(glId).toBeTruthy(); + const box = await page.locator(`#${glId}`).boundingBox(); + expect(box).toBeTruthy(); + + const { vp, pos } = await page.evaluate(() => { + const w = window as unknown as LocksWindow; + return { + vp: JSON.parse(w.Module.kicadCollabGetViewport()), + pos: w.Module.kicadCollabGetPos("22222222-0000-0000-0000-000000000001"), + }; + }); + const [wx, wy] = pos.split(",").map(Number); + return { + x: box!.x + (wx - vp.cx) * vp.scale + vp.w / 2, + y: box!.y + (wy - vp.cy) * vp.scale + vp.h / 2, + }; +} + +async function dragFromWire(page: Page): Promise { + const at = await wireScreenPos(page); + await page.mouse.move(at.x, at.y); + await page.mouse.down(); + await page.mouse.move(at.x + 100, at.y + 60, { steps: 10 }); + await page.mouse.up(); + await page.waitForTimeout(800); +} + +test("a locked wire resists a real drag; the same drag moves it unlocked", async ({ + page, + testLogger, +}) => { + test.setTimeout(240000); + await bootAndOpen(page); + + await setLock(page, true); + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabTestGetLocked()), + ), + ) + .toEqual([{ uuid: WIRE1, name: "bob" }]); + + const before = await wirePos(page); + expect(before).toBeTruthy(); + await dragFromWire(page); + expect(await wirePos(page)).toBe(before); + + // Control: unlocked, the same gesture moves/bends the wire. + await setLock(page, false); + await dragFromWire(page); + await expect + .poll(() => wirePos(page), { + timeout: 10000, + message: "control drag never moved the unlocked wire", + }) + .not.toBe(before); + + expect(hasAbort(testLogger)).toBe(false); +}); + +test("a locked wire stays selectable; ReleaseSelection strips it", async ({ + page, + testLogger, +}) => { + test.setTimeout(240000); + await bootAndOpen(page); + await setLock(page, true); + + // Inspection: programmatic select through the real tool still lands. + const id = await page.evaluate(() => + (window as unknown as LocksWindow).Module.kicadCollabTestSelectFirst(), + ); + expect(id).toBeTruthy(); + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabGetSelection()), + ), + ) + .toContain(id); + + // Tiebreak-loser path: release exactly that uuid. + await page.evaluate( + ({ uuid }) => + (window as unknown as LocksWindow).Module.kicadCollabReleaseSelection( + JSON.stringify([uuid]), + "bob", + ), + { uuid: id }, + ); + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabGetSelection()), + ), + ) + .toEqual([]); + + expect(hasAbort(testLogger)).toBe(false); +}); diff --git a/tests/kicad/presence-locks-pcbnew.spec.ts b/tests/kicad/presence-locks-pcbnew.spec.ts new file mode 100644 index 0000000..d161d9b --- /dev/null +++ b/tests/kicad/presence-locks-pcbnew.spec.ts @@ -0,0 +1,316 @@ +import type { Page } from "@playwright/test"; +import { test, expect } from "./fixtures"; + +/** + * pcbnew selection soft-locks — C++ veto e2e (collab-presence 0007). + * + * Seeds the remote lock set directly via kicadCollabSetRemote (`locks` field) + * and drives REAL input against it: + * - a drag-move gesture starting on a locked footprint must not move it + * (selectPoint's aOnDrag strip + the move request filter), while the SAME + * gesture moves it once the lock clears (the control that proves the + * gesture itself works); + * - plain selection of a locked item stays possible (inspection semantics); + * - kicadCollabReleaseSelection unselects exactly the contested uuids + * (the tiebreak-loser path), leaving the rest selected; + * - kicadCollabTestGetLocked mirrors the seeded set. + * + * The awareness/tiebreak loop lives in TS (unit-tested) and in the two-tab + * tests/web/locks.spec.ts. + */ + +const SEG1 = "44444444-0000-0000-0000-000000000001"; +const FP1 = "66666666-0000-0000-0000-000000000001"; +const SAMPLE_PCB = `(kicad_pcb +\t(version 20241229) +\t(generator "pcbnew") +\t(generator_version "9.0") +\t(general +\t\t(thickness 1.6) +\t) +\t(paper "A4") +\t(layers +\t\t(0 "F.Cu" signal) +\t\t(2 "B.Cu" signal) +\t\t(37 "F.SilkS" user) +\t\t(25 "Edge.Cuts" user) +\t) +\t(setup) +\t(net 0 "") +\t(footprint "TestLib:R" +\t\t(layer "F.Cu") +\t\t(uuid "${FP1}") +\t\t(at 100 100) +\t\t(attr smd) +\t\t(fp_rect (start -2 -2) (end 2 2) (layer "F.SilkS") (stroke (width 0.3) (type solid)) (uuid "66666666-0000-0000-0000-0000000000cc")) +\t\t(pad "1" smd rect +\t\t\t(at 0 0) +\t\t\t(size 3 3) +\t\t\t(layers "F.Cu") +\t\t\t(uuid "66666666-0000-0000-0000-0000000000dd") +\t\t) +\t) +\t(segment (start 50.8 50.8) (end 101.6 50.8) (width 0.2) (layer "F.Cu") (net 0) (uuid "${SEG1}")) +) +`; + +type FS = { mkdirTree(p: string): void; writeFile(p: string, d: string): void }; +type Mod = { + kicadOpenFile(p: string): unknown; + kicadCollabPresenceStart(): void; + kicadCollabSetRemote(j: string): void; + kicadCollabGetViewport(): string; + kicadCollabGetSelection(): string; + kicadCollabGetPos(id: string): string; + kicadCollabReleaseSelection(uuidsJson: string, holder: string): void; + kicadCollabTestGetLocked(): string; + kicadCollabTestSelectComponent(): string; + kicadCollabTestSelectFirst(): string; + kicadCollabTestClearSelection(): boolean; +}; +type LocksWindow = { + FS: FS; + Module: Mod; + kicadCollab?: Record; +}; + +function hasAbort(l: { consoleLogs: string[]; errors: string[] }): boolean { + return [...l.consoleLogs, ...l.errors].some((s) => s.includes("Aborted(")); +} + +async function bootAndOpen(page: Page): Promise { + await page.goto("/kicad/pcbnew-collab.html"); + await expect(page.locator("#canvas")).toBeVisible({ timeout: 90000 }); + await page.waitForFunction(() => !!window.wxElementRegistry, null, { timeout: 90000 }); + await page.waitForFunction( + () => { + const m = (window as unknown as { Module?: Partial }).Module; + return ( + typeof m?.kicadOpenFile === "function" && + typeof m?.kicadCollabTestGetLocked === "function" + ); + }, + null, + { timeout: 90000 }, + ); + await page.waitForFunction( + () => + !!window.wxElementRegistry && + window.wxElementRegistry + .findAll({ visible: true }) + .some((e) => /Frame$/.test(e.typeName) || (e.name || "").endsWith("Frame")), + null, + { timeout: 90000 }, + ); + + await page.evaluate( + ({ content }) => { + const w = window as unknown as LocksWindow; + const dir = "/home/kicad/documents"; + try { + w.FS.mkdirTree(dir); + } catch { + /* exists */ + } + const p = `${dir}/locks.kicad_pcb`; + w.FS.writeFile(p, content); + w.Module.kicadOpenFile(p); + }, + { content: SAMPLE_PCB }, + ); + + await expect + .poll(() => page.title(), { timeout: 60000, intervals: [500] }) + .toMatch(/locks/i); + + await page.evaluate(() => { + (window as unknown as LocksWindow).Module.kicadCollabPresenceStart(); + }); +} + +/** Seed (or clear) the remote lock set for FP1. */ +function setLock(page: Page, locked: boolean): Promise { + return page.evaluate( + ({ fp, locked: isLocked }) => { + const w = window as unknown as LocksWindow; + w.Module.kicadCollabSetRemote( + JSON.stringify({ + peers: [], + locks: isLocked ? [{ uuid: fp, name: "bob" }] : [], + }), + ); + }, + { fp: FP1, locked }, + ); +} + +/** The footprint's current screen position via the exported GAL transform. */ +async function fpScreenPos(page: Page): Promise<{ x: number; y: number }> { + const glId = await page.evaluate(() => { + const visible = Array.from(document.querySelectorAll('[id^="glcanvas-"]')) + .map((c) => c as HTMLCanvasElement) + .find( + (c) => + window.getComputedStyle(c).display !== "none" && + c.getBoundingClientRect().width > 0, + ); + return visible?.id ?? null; + }); + expect(glId).toBeTruthy(); + const box = await page.locator(`#${glId}`).boundingBox(); + expect(box).toBeTruthy(); + + const { vp, pos } = await page.evaluate(() => { + const w = window as unknown as LocksWindow; + return { + vp: JSON.parse(w.Module.kicadCollabGetViewport()), + pos: w.Module.kicadCollabGetPos("66666666-0000-0000-0000-000000000001"), + }; + }); + const [wx, wy] = pos.split(",").map(Number); + return { + x: box!.x + (wx - vp.cx) * vp.scale + vp.w / 2, + y: box!.y + (wy - vp.cy) * vp.scale + vp.h / 2, + }; +} + +const fpPos = (page: Page) => + page.evaluate(() => + (window as unknown as LocksWindow).Module.kicadCollabGetPos( + "66666666-0000-0000-0000-000000000001", + ), + ); + +/** + * The real pcbnew move flow: click-select the footprint, then the `M` hotkey + * (pcbnew's default LEFT-DRAG gesture is rubber-band select, not move — the + * eeschema spec covers the drag-gesture path where dragging DOES move). + * `M` routes through EDIT_TOOL::Move → RequestSelection → the locked-items + * client filter — exactly the veto under test. The drop is a click at the + * offset position. + */ +async function moveViaHotkey(page: Page): Promise { + const at = await fpScreenPos(page); + await page.mouse.click(at.x, at.y); // select (allowed — inspection semantics) + await page.waitForTimeout(500); + await page.mouse.move(at.x, at.y); + await page.keyboard.press("m"); + await page.waitForTimeout(500); + await page.mouse.move(at.x + 120, at.y + 80, { steps: 10 }); + await page.mouse.click(at.x + 120, at.y + 80); // drop (no-op if move never started) + await page.waitForTimeout(800); + await page.keyboard.press("Escape"); // leave no half-open tool between phases + await page.waitForTimeout(300); +} + +test("seeded locks are probeable and a locked footprint resists a real move", async ({ + page, + testLogger, +}) => { + test.setTimeout(240000); + await bootAndOpen(page); + + await setLock(page, true); + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabTestGetLocked()), + ), + ) + .toEqual([{ uuid: FP1, name: "bob" }]); + + // Veto: select + M + drop must leave the locked footprint in place. + const before = await fpPos(page); + expect(before).toBeTruthy(); + await moveViaHotkey(page); + expect(await fpPos(page)).toBe(before); + + // Control: the SAME flow moves it once the lock clears — proving the veto + // (not a broken gesture) kept it in place above. + await setLock(page, false); + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabTestGetLocked()), + ), + ) + .toEqual([]); + await moveViaHotkey(page); + await expect + .poll(() => fpPos(page), { + timeout: 10000, + message: "control move never moved the unlocked footprint", + }) + .not.toBe(before); + + expect(hasAbort(testLogger)).toBe(false); +}); + +test("a locked item stays selectable for inspection", async ({ page, testLogger }) => { + test.setTimeout(240000); + await bootAndOpen(page); + await setLock(page, true); + + // Programmatic select drives the real selection tool (no drag involved). + const id = await page.evaluate(() => + (window as unknown as LocksWindow).Module.kicadCollabTestSelectComponent(), + ); + expect(id).toBe(FP1); + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabGetSelection()), + ), + ) + .toContain(FP1); + + expect(hasAbort(testLogger)).toBe(false); +}); + +test("ReleaseSelection unselects exactly the contested uuids", async ({ + page, + testLogger, +}) => { + test.setTimeout(240000); + await bootAndOpen(page); + + // Select the footprint plus (if distinct) the first top-level item through + // the real tool — TestSelectFirst may pick the footprint itself, so assert + // set-minus semantics rather than counts. + await page.evaluate(() => { + const w = window as unknown as LocksWindow; + w.Module.kicadCollabTestSelectComponent(); + w.Module.kicadCollabTestSelectFirst(); + }); + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabGetSelection()), + ), + ) + .toContain(FP1); + const preRelease: string[] = await page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabGetSelection()), + ); + + // Lose the tiebreak on the footprint only. + await page.evaluate( + ({ fp }) => + (window as unknown as LocksWindow).Module.kicadCollabReleaseSelection( + JSON.stringify([fp]), + "bob", + ), + { fp: FP1 }, + ); + + // Exactly the contested uuid drops; anything else selected survives. + await expect + .poll(() => + page.evaluate(() => + JSON.parse((window as unknown as LocksWindow).Module.kicadCollabGetSelection()), + ), + ) + .toEqual(preRelease.filter((u) => u !== FP1)); + + expect(hasAbort(testLogger)).toBe(false); +}); diff --git a/tests/web/locks.spec.ts b/tests/web/locks.spec.ts new file mode 100644 index 0000000..6531351 --- /dev/null +++ b/tests/web/locks.spec.ts @@ -0,0 +1,111 @@ +import { test, expect, type Page } from '@playwright/test'; + +/** + * Selection soft-locks e2e (collab-presence 0007): two tabs of the real app + * on the SAME board share the per-file room — one tab's live selection must + * soft-lock those items for the other, and an overlapping hold (both grab the + * same item inside the propagation window — simulated with the programmatic + * select, which bypasses the acquisition veto exactly like a real race) + * resolves by the deterministic (user.id, clientID) tiebreak: alice keeps, + * bob's client releases. + */ + +const SCOPE = 'default'; + +type Mod = { + kicadCollabGetSelection(): string; + kicadCollabTestGetLocked(): string; + kicadCollabTestSelectComponent(): string; + kicadCollabTestClearSelection(): boolean; +}; +type W = { Module: Mod }; + +async function bootBoard(page: Page, user: string): Promise { + await page.goto(`/${SCOPE}/projects/demo/demo.kicad_pcb?user=${user}`); + await expect(page.locator('#canvas')).toBeVisible({ timeout: 120000 }); + await expect + .poll(() => page.title(), { + message: `${user}: board editor never reached the expected title`, + timeout: 120000, + intervals: [1000], + }) + .toMatch(/demo — PCB Editor/i); + await page.waitForFunction( + () => + typeof (window as unknown as Partial).Module?.kicadCollabTestGetLocked === + 'function', + null, + { timeout: 60000 }, + ); +} + +const selection = (page: Page) => + page.evaluate(() => + JSON.parse((window as unknown as W).Module.kicadCollabGetSelection()), + ); +const locked = (page: Page) => + page.evaluate(() => + JSON.parse((window as unknown as W).Module.kicadCollabTestGetLocked()), + ); + +test('a peer selection locks the item; overlapping holds tiebreak deterministically', async ({ + page, + context, +}) => { + test.setTimeout(480000); // two full board boots + + const alice = page; + await bootBoard(alice, 'alice'); + const bob = await context.newPage(); + await bootBoard(bob, 'bob'); + + // ── lock propagation ─────────────────────────────────────────────────────── + const fpId = await alice.evaluate(() => + (window as unknown as W).Module.kicadCollabTestSelectComponent(), + ); + expect(fpId, 'demo board should contain a footprint').toBeTruthy(); + + await expect + .poll(() => locked(bob), { + timeout: 20000, + message: "bob never saw alice's selection as a lock", + }) + .toEqual([{ uuid: fpId, name: 'alice' }]); + // The holder's own tab is NOT locked against itself. + await expect.poll(() => locked(alice), { timeout: 20000 }).toEqual([]); + + // ── overlapping hold → tiebreak ──────────────────────────────────────────── + // bob grabs the SAME footprint programmatically (the race-window simulation: + // AddItemToSel bypasses the acquisition veto, like two grabs inside the + // awareness propagation window). + const bobPick = await bob.evaluate(() => + (window as unknown as W).Module.kicadCollabTestSelectComponent(), + ); + expect(bobPick).toBe(fpId); + + // alice ("alice" < "bob") keeps it; bob's client releases it. + await expect + .poll(() => selection(bob), { + timeout: 20000, + message: "bob's losing hold was never released", + }) + .toEqual([]); + expect(await selection(alice)).toContain(fpId); + + // ── unlock on clear ──────────────────────────────────────────────────────── + await alice.evaluate(() => + (window as unknown as W).Module.kicadCollabTestClearSelection(), + ); + await expect.poll(() => locked(bob), { timeout: 20000 }).toEqual([]); + + // Now bob's grab sticks, and it locks the item for alice. + await bob.evaluate(() => + (window as unknown as W).Module.kicadCollabTestSelectComponent(), + ); + await expect + .poll(() => selection(bob), { timeout: 20000 }) + .toContain(fpId); + await expect + .poll(() => locked(alice), { timeout: 20000 }) + .toEqual([{ uuid: fpId, name: 'bob' }]); +}); diff --git a/wasm/bindings/eeschema_embind.cpp b/wasm/bindings/eeschema_embind.cpp index 95fcdd6..573e7bd 100644 --- a/wasm/bindings/eeschema_embind.cpp +++ b/wasm/bindings/eeschema_embind.cpp @@ -53,7 +53,9 @@ #include #include #include +#include #include +#include #include "collab_presence_style.h" #include @@ -720,6 +722,8 @@ struct PIN std::vector g_peers; std::vector g_pins; +// Remote soft-locks (collab-presence 0007) — see pcbnew_embind.cpp. +std::map g_locks; // Every visual knob (shapes, widths, alphas, label placement, color overrides) // — see collab_presence_style.h; live-patched by kicadCollabSetStyle (tuner). // eeschema ships its own defaults (hairline outline, subtler fill/cursor). @@ -1573,6 +1577,22 @@ void schCollabPresenceStart() presence::g_started = true; + // Remote soft-locks (0007): let the selection/move tools consult the + // peers' live selections through the fork's process-global query. + PCBJAM_REMOTE_LOCK::SetQuery( + []( const KIID& aId, wxString* aHolder ) -> bool + { + auto it = presence::g_locks.find( aId ); + + if( it == presence::g_locks.end() ) + return false; + + if( aHolder ) + *aHolder = wxString::FromUTF8( it->second.c_str() ); + + return true; + } ); + wxWindow* canvas = fr->GetCanvas(); canvas->Bind( wxEVT_MOTION, []( wxMouseEvent& e ) { presence::onMotion( e ); } ); @@ -1633,7 +1653,19 @@ void schCollabSetRemote( std::string aJson ) peers.push_back( std::move( peer ) ); } + // Remote soft-locks (0007): `locks: [{uuid, name}]` — see pcbnew_embind.cpp. + std::map locks; + + for( const json& l : j.value( "locks", json::array() ) ) + { + std::string uuid = l.is_object() ? l.value( "uuid", "" ) : ""; + + if( !uuid.empty() ) + locks[ KIID( wxString::FromUTF8( uuid.c_str() ) ) ] = l.value( "name", "" ); + } + presence::g_peers = std::move( peers ); + presence::g_locks = std::move( locks ); schCollabPresenceStart(); presence::scheduleRedraw(); } @@ -1930,6 +1962,79 @@ std::string schCollabTestSelectComponent() return toUtf8( target->m_Uuid.AsString() ); } +// JS → C++ (0007): tiebreak release — see pcbnew_embind.cpp for the design +// (cancel-interactive-if-a-tool-holds-them → selective unselect → infobar → +// forced re-emit). +void schCollabReleaseSelection( std::string aUuidsJson, std::string aHolder ) +{ + json j = json::parse( aUuidsJson, nullptr, /*allow_exceptions*/ false ); + + if( j.is_discarded() || !j.is_array() ) + return; + + SCH_EDIT_FRAME* fr = schFrame(); + + if( !fr ) + return; + + std::vector ids; + + for( const json& u : j ) + { + if( u.is_string() ) + ids.emplace_back( wxString::FromUTF8( u.get().c_str() ) ); + } + + if( ids.empty() ) + return; + + wxString holder = wxString::FromUTF8( aHolder.c_str() ); + + fr->CallAfter( [fr, ids, holder]() { + if( !fr->ToolStackIsEmpty() ) + fr->GetToolManager()->RunAction( ACTIONS::cancelInteractive ); + + SCH_SELECTION_TOOL* st = fr->GetToolManager()->GetTool(); + + if( !st ) + return; + + bool released = false; + + for( const KIID& id : ids ) + { + SCH_ITEM* item = fr->Schematic().ResolveItem( id, nullptr, /*allowNull*/ true ); + + if( item && item->IsSelected() ) + { + st->RemoveItemFromSel( item ); + released = true; + } + } + + if( released ) + { + fr->ShowInfoBarWarning( wxString::Format( _( "%s is editing this — released from " + "your selection." ), + holder ), + true ); + } + + schedulePresenceSelCheck(); + } ); +} + +// Test probe (0007): the current remote soft-lock set as `[{uuid, name}]`. +std::string schCollabTestGetLocked() +{ + json arr = json::array(); + + for( const auto& [id, name] : presence::g_locks ) + arr.push_back( { { "uuid", toUtf8( id.AsString() ) }, { "name", name } } ); + + return arr.dump(); +} + // Test helper: clear the selection through the tool + run the presence check. bool schCollabTestClearSelection() { @@ -2024,6 +2129,9 @@ EMSCRIPTEN_BINDINGS(eeschema) { // Cross-app selection (0006). function("kicadCollabGetSelectionFull", &schCollabGetSelectionFull); function("kicadCollabTestGetCrossMapped", &schCollabTestGetCrossMapped); + // Selection soft-locks (0007). + function("kicadCollabReleaseSelection", &schCollabReleaseSelection); + function("kicadCollabTestGetLocked", &schCollabTestGetLocked); function("kicadCollabTestSelectFirst", &schCollabTestSelectFirst); function("kicadCollabTestSelectComponent", &schCollabTestSelectComponent); function("kicadCollabTestClearSelection", &schCollabTestClearSelection); diff --git a/wasm/bindings/kicad_editor_embind.cpp b/wasm/bindings/kicad_editor_embind.cpp index 7b7cfe7..2ce7c8d 100644 --- a/wasm/bindings/kicad_editor_embind.cpp +++ b/wasm/bindings/kicad_editor_embind.cpp @@ -59,6 +59,9 @@ std::string pcbCollabGetSelection(); std::string pcbCollabGetSelectionFull(); std::string pcbCollabTestGetCrossMapped(); std::string pcbCollabTestSelectComponent(); +// Selection soft-locks (collab-presence 0007). +void pcbCollabReleaseSelection( std::string aUuidsJson, std::string aHolder ); +std::string pcbCollabTestGetLocked(); std::string pcbCollabTestSelectFirst(); bool pcbCollabTestClearSelection(); @@ -85,6 +88,9 @@ std::string schCollabGetSelection(); std::string schCollabGetSelectionFull(); std::string schCollabTestGetCrossMapped(); std::string schCollabTestSelectComponent(); +// Selection soft-locks (collab-presence 0007). +void schCollabReleaseSelection( std::string aUuidsJson, std::string aHolder ); +std::string schCollabTestGetLocked(); std::string schCollabTestSelectFirst(); bool schCollabTestClearSelection(); @@ -227,6 +233,17 @@ static std::string collabTestSelectComponent() return pcbEditorActive() ? pcbCollabTestSelectComponent() : schCollabTestSelectComponent(); } +static void collabReleaseSelection( std::string aUuidsJson, std::string aHolder ) +{ + pcbEditorActive() ? pcbCollabReleaseSelection( aUuidsJson, aHolder ) + : schCollabReleaseSelection( aUuidsJson, aHolder ); +} + +static std::string collabTestGetLocked() +{ + return pcbEditorActive() ? pcbCollabTestGetLocked() : schCollabTestGetLocked(); +} + static std::string collabTestSelectFirst() { return pcbEditorActive() ? pcbCollabTestSelectFirst() : schCollabTestSelectFirst(); @@ -268,6 +285,9 @@ EMSCRIPTEN_BINDINGS(kicad_editor) { function("kicadCollabGetSelectionFull", &collabGetSelectionFull); function("kicadCollabTestGetCrossMapped", &collabTestGetCrossMapped); function("kicadCollabTestSelectComponent", &collabTestSelectComponent); + // Selection soft-locks (collab-presence 0007). + function("kicadCollabReleaseSelection", &collabReleaseSelection); + function("kicadCollabTestGetLocked", &collabTestGetLocked); function("kicadCollabTestSelectFirst", &collabTestSelectFirst); function("kicadCollabTestClearSelection", &collabTestClearSelection); } diff --git a/wasm/bindings/pcbnew_embind.cpp b/wasm/bindings/pcbnew_embind.cpp index 8dd82a9..797dfc1 100644 --- a/wasm/bindings/pcbnew_embind.cpp +++ b/wasm/bindings/pcbnew_embind.cpp @@ -40,8 +40,10 @@ #include #include #include +#include #include #include +#include #include #include #include @@ -1126,6 +1128,12 @@ struct PIN std::vector g_peers; std::vector g_pins; +// Remote soft-locks (collab-presence 0007): uuid → holding peer's display +// name, derived by the TS side from ALL other clients' live selections (own +// user's other tabs included). Consulted by the fork's PCBJAM_REMOTE_LOCK +// query from the selection/move tools. Ephemeral — replaced on every +// kicadCollabSetRemote snapshot. +std::map g_locks; // Every visual knob (shapes, widths, alphas, label placement, color overrides) // — see collab_presence_style.h; live-patched by kicadCollabSetStyle (tuner). pcbjam_presence::STYLE g_style; @@ -1709,6 +1717,22 @@ void pcbCollabPresenceStart() presence::g_started = true; + // Remote soft-locks (0007): let the selection/move tools consult the + // peers' live selections through the fork's process-global query. + PCBJAM_REMOTE_LOCK::SetQuery( + []( const KIID& aId, wxString* aHolder ) -> bool + { + auto it = presence::g_locks.find( aId ); + + if( it == presence::g_locks.end() ) + return false; + + if( aHolder ) + *aHolder = wxString::FromUTF8( it->second.c_str() ); + + return true; + } ); + wxWindow* canvas = fr->GetCanvas(); canvas->Bind( wxEVT_MOTION, []( wxMouseEvent& e ) { presence::onMotion( e ); } ); @@ -1770,7 +1794,20 @@ void pcbCollabSetRemote( std::string aJson ) peers.push_back( std::move( peer ) ); } + // Remote soft-locks (0007): `locks: [{uuid, name}]` — every other client's + // held uuids with the holder's display name for the infobar. + std::map locks; + + for( const json& l : j.value( "locks", json::array() ) ) + { + std::string uuid = l.is_object() ? l.value( "uuid", "" ) : ""; + + if( !uuid.empty() ) + locks[ KIID( wxString::FromUTF8( uuid.c_str() ) ) ] = l.value( "name", "" ); + } + presence::g_peers = std::move( peers ); + presence::g_locks = std::move( locks ); pcbCollabPresenceStart(); presence::scheduleRedraw(); } @@ -2061,6 +2098,82 @@ std::string pcbCollabTestSelectComponent() return toUtf8( target->m_Uuid.AsString() ); } +// JS → C++ (0007): the local client LOST the selection tiebreak — release the +// contested items (only those) from the live selection. If an interactive +// tool (move/drag) holds them, cancel it first (ESC semantics — the preview +// reverts); a bare cancel is NOT sent when idle, since ESC on the base +// selection tool would clear the whole selection. Ends with a forced +// selection re-emit (programmatic changes close no canvas event). +void pcbCollabReleaseSelection( std::string aUuidsJson, std::string aHolder ) +{ + json j = json::parse( aUuidsJson, nullptr, /*allow_exceptions*/ false ); + + if( j.is_discarded() || !j.is_array() ) + return; + + PCB_EDIT_FRAME* fr = pcbFrame(); + + if( !fr ) + return; + + std::vector ids; + + for( const json& u : j ) + { + if( u.is_string() ) + ids.emplace_back( wxString::FromUTF8( u.get().c_str() ) ); + } + + if( ids.empty() ) + return; + + wxString holder = wxString::FromUTF8( aHolder.c_str() ); + + fr->CallAfter( [fr, ids, holder]() { + if( !fr->ToolStackIsEmpty() ) + fr->GetToolManager()->RunAction( ACTIONS::cancelInteractive ); + + PCB_SELECTION_TOOL* st = fr->GetToolManager()->GetTool(); + + if( !st ) + return; + + bool released = false; + + for( const KIID& id : ids ) + { + BOARD_ITEM* item = fr->GetBoard()->ResolveItem( id, /*allowNullptr*/ true ); + + if( item && item->IsSelected() ) + { + st->RemoveItemFromSel( item ); + released = true; + } + } + + if( released ) + { + fr->ShowInfoBarWarning( wxString::Format( _( "%s is editing this — released from " + "your selection." ), + holder ), + true ); + } + + schedulePresenceSelCheck(); + } ); +} + +// Test probe (0007): the current remote soft-lock set as `[{uuid, name}]`. +std::string pcbCollabTestGetLocked() +{ + json arr = json::array(); + + for( const auto& [id, name] : presence::g_locks ) + arr.push_back( { { "uuid", toUtf8( id.AsString() ) }, { "name", name } } ); + + return arr.dump(); +} + // Test helper: clear the selection through the tool + run the presence check. bool pcbCollabTestClearSelection() { @@ -2338,6 +2451,9 @@ EMSCRIPTEN_BINDINGS(pcbnew) { // Cross-app selection (0006). function("kicadCollabGetSelectionFull", &pcbCollabGetSelectionFull); function("kicadCollabTestGetCrossMapped", &pcbCollabTestGetCrossMapped); + // Selection soft-locks (0007). + function("kicadCollabReleaseSelection", &pcbCollabReleaseSelection); + function("kicadCollabTestGetLocked", &pcbCollabTestGetLocked); function("kicadCollabTestSelectFirst", &pcbCollabTestSelectFirst); function("kicadCollabTestSelectComponent", &pcbCollabTestSelectComponent); function("kicadCollabTestClearSelection", &pcbCollabTestClearSelection); diff --git a/web/standalone/src/wasm/collab/cross-app.test.ts b/web/standalone/src/wasm/collab/cross-app.test.ts index 0e56577..04ee828 100644 --- a/web/standalone/src/wasm/collab/cross-app.test.ts +++ b/web/standalone/src/wasm/collab/cross-app.test.ts @@ -30,10 +30,14 @@ async function join(projectId: string, userId: string, tool: string): Promise { +afterEach(async () => { for (const h of handles) h.destroy(); handles = []; resetPresenceColorClaims(); + // Drain already-queued BroadcastChannel deliveries before vitest recycles + // the module registry — a late message otherwise trips the vite-node module + // proxy (flaky unhandled RangeError at the schema getter). + await new Promise((r) => setTimeout(r, 20)); }); describe("startCrossAppPresence", () => { diff --git a/web/standalone/src/wasm/collab/lock-tiebreak.test.ts b/web/standalone/src/wasm/collab/lock-tiebreak.test.ts new file mode 100644 index 0000000..4cb8001 --- /dev/null +++ b/web/standalone/src/wasm/collab/lock-tiebreak.test.ts @@ -0,0 +1,74 @@ +import { describe, expect, it } from "vitest"; +import { beats, contestedReleases, remoteLocks, type LockClient } from "./lock-tiebreak"; + +/** Soft-lock tiebreak policy unit tests (collab-presence 0007). */ + +function client(userId: string, clientId: number, selection: string[]): LockClient { + return { userId, clientId, name: userId, selection }; +} + +describe("beats", () => { + it("orders lexicographically on (userId, clientId)", () => { + expect(beats({ userId: "alice", clientId: 9 }, { userId: "bob", clientId: 1 })).toBe(true); + expect(beats({ userId: "bob", clientId: 1 }, { userId: "alice", clientId: 9 })).toBe(false); + // Same user's two tabs fall to clientId. + expect(beats({ userId: "alice", clientId: 1 }, { userId: "alice", clientId: 2 })).toBe(true); + expect(beats({ userId: "alice", clientId: 2 }, { userId: "alice", clientId: 1 })).toBe(false); + }); +}); + +describe("remoteLocks", () => { + const self = { userId: "mid", clientId: 50, selection: [] as string[] }; + + it("unions every other client's selection with the winning holder's name", () => { + const locks = remoteLocks(self, [ + client("zed", 9, ["u1", "u2"]), + client("alice", 1, ["u2"]), // alice beats zed → she is u2's holder + ]); + expect(locks).toEqual([ + { uuid: "u1", name: "zed" }, + { uuid: "u2", name: "alice" }, + ]); + }); + + it("excludes uuids the local client holds AND wins, keeps ones it loses", () => { + const locks = remoteLocks({ userId: "mid", clientId: 50, selection: ["won", "lost"] }, [ + client("zed", 9, ["won"]), // mid < zed → local wins → not locked for us + client("alice", 1, ["lost"]), // alice < mid → local loses → stays locked + ]); + expect(locks).toEqual([{ uuid: "lost", name: "alice" }]); + }); + + it("is empty with no other clients", () => { + expect(remoteLocks(self, [])).toEqual([]); + }); +}); + +describe("contestedReleases", () => { + it("releases only uuids a WINNING peer also holds, blaming the strongest", () => { + const self = { userId: "mid", clientId: 50, selection: ["a", "b", "c"] }; + const release = contestedReleases(self, [ + client("alice", 1, ["a"]), // wins → release a + client("zed", 9, ["b"]), // loses → keep b (zed releases, not us) + client("bob", 2, ["c"]), // wins → release c + ]); + expect(release).toEqual({ uuids: ["a", "c"], holder: "alice" }); + }); + + it("returns null when nothing is contested or every contest is won", () => { + const self = { userId: "alice", clientId: 1, selection: ["a"] }; + expect(contestedReleases(self, [client("bob", 2, ["a"])])).toBeNull(); + expect(contestedReleases(self, [client("bob", 2, ["other"])])).toBeNull(); + expect(contestedReleases({ ...self, selection: [] }, [client("bob", 2, ["a"])])).toBeNull(); + }); + + it("own-user other tab contests fall to clientId", () => { + const tab2 = { userId: "alice", clientId: 7, selection: ["a"] }; + expect(contestedReleases(tab2, [client("alice", 3, ["a"])])).toEqual({ + uuids: ["a"], + holder: "alice", + }); + const tab1 = { userId: "alice", clientId: 3, selection: ["a"] }; + expect(contestedReleases(tab1, [client("alice", 7, ["a"])])).toBeNull(); + }); +}); diff --git a/web/standalone/src/wasm/collab/lock-tiebreak.ts b/web/standalone/src/wasm/collab/lock-tiebreak.ts new file mode 100644 index 0000000..a41857b --- /dev/null +++ b/web/standalone/src/wasm/collab/lock-tiebreak.ts @@ -0,0 +1,89 @@ +/** + * Selection soft-lock policy (collab-presence 0007) — pure logic, no I/O. + * + * Lock rule: an item selected by any OTHER awareness client (the same user's + * other tab included) is soft-locked locally — still selectable for + * inspection, filtered from move/drag acquisition (enforced C++-side via the + * fork's PCBJAM_REMOTE_LOCK query, fed from `remoteLocks`). + * + * Tiebreak rule (overlapping holds — both grabbed inside the awareness + * propagation window): lowest `(user.id, clientID)` lexicographic KEEPS the + * item; every losing client releases it (`contestedReleases` → the C++ + * `kicadCollabReleaseSelection` entry point). Pure and computable from + * awareness state alone, so all clients agree with no coordination and no + * oscillation. Known accepted bias: alphabetically-early users win. + */ + +export interface LockIdentity { + userId: string; + clientId: number; +} + +export interface LockClient extends LockIdentity { + /** Display name for the infobar ("R5 is being edited by "). */ + name: string; + selection: string[]; +} + +/** True when `a` beats `b` in the deterministic (userId, clientId) order. */ +export function beats(a: LockIdentity, b: LockIdentity): boolean { + if (a.userId !== b.userId) return a.userId < b.userId; + return a.clientId < b.clientId; +} + +/** + * The soft-lock set to feed the C++ query: every uuid held by another client, + * EXCEPT uuids the local client holds and WINS — the winner must not be + * blocked from moving its own item while the loser's release is in flight. + * (Uuids the local client holds and LOSES stay locked: the release is about + * to strip them anyway, and moving them meanwhile would fight the winner.) + * Holder name = the winning holder when several clients hold the same uuid. + */ +export function remoteLocks( + self: LockIdentity & { selection: string[] }, + others: LockClient[], +): Array<{ uuid: string; name: string }> { + const ownHeld = new Set(self.selection); + const byUuid = new Map(); + + for (const client of others) { + for (const uuid of client.selection) { + const current = byUuid.get(uuid); + if (!current || beats(client, current)) byUuid.set(uuid, client); + } + } + + const out: Array<{ uuid: string; name: string }> = []; + for (const [uuid, holder] of byUuid) { + if (ownHeld.has(uuid) && beats(self, holder)) continue; + out.push({ uuid, name: holder.name }); + } + return out.sort((a, b) => a.uuid.localeCompare(b.uuid)); +} + +/** + * The uuids the LOCAL client must release because a winning peer also holds + * them, plus the (single) name to blame in the infobar — the strongest + * winning holder when several are involved. + */ +export function contestedReleases( + self: LockIdentity & { selection: string[] }, + others: LockClient[], +): { uuids: string[]; holder: string } | null { + const uuids: string[] = []; + let strongest: LockClient | null = null; + + for (const uuid of self.selection) { + let winner: LockClient | null = null; + for (const client of others) { + if (!client.selection.includes(uuid)) continue; + if (beats(client, self) && (!winner || beats(client, winner))) winner = client; + } + if (winner) { + uuids.push(uuid); + if (!strongest || beats(winner, strongest)) strongest = winner; + } + } + + return uuids.length && strongest ? { uuids, holder: strongest.name } : null; +} diff --git a/web/standalone/src/wasm/collab/presence-kicad.test.ts b/web/standalone/src/wasm/collab/presence-kicad.test.ts index 77de47c..e59d35d 100644 --- a/web/standalone/src/wasm/collab/presence-kicad.test.ts +++ b/web/standalone/src/wasm/collab/presence-kicad.test.ts @@ -27,8 +27,14 @@ function fakeModule() { function stubPresence(peers: PresencePeer[] = []) { const subscribers = new Set<(p: PresencePeer[]) => void>(); - const handle: PresenceHandle & { firePeers(p: PresencePeer[]): void } = { + let clients: PresencePeer[] = peers; + const handle: PresenceHandle & { + firePeers(p: PresencePeer[]): void; + fireClients(c: PresencePeer[]): void; + } = { peers: () => peers, + clients: () => clients, + self: () => ({ userId: "local", clientId: 100 }), subscribe(cb) { subscribers.add(cb); return () => subscribers.delete(cb); @@ -39,8 +45,13 @@ function stubPresence(peers: PresencePeer[] = []) { destroy: vi.fn(), firePeers(p) { peers = p; + clients = p; for (const cb of subscribers) cb(p); }, + fireClients(c) { + clients = c; + for (const cb of subscribers) cb(peers); + }, }; return handle; } @@ -195,6 +206,63 @@ describe("xselFromPeerState", () => { }); }); +describe("bindKicadPresence × soft-locks (0007)", () => { + it("pushes the locks set derived from ALL other clients", async () => { + const mod = fakeModule(); + const presence = stubPresence(); + bindKicadPresence({ mod, win: {}, presence }); + + presence.fireClients([ + peer("bob", { clientId: 7, selection: ["u1"] }), + // Own user's other tab locks too (clients() is not user-deduped). + peer("local", { clientId: 8, selection: ["u2"] }), + ]); + await new Promise((r) => setTimeout(r, 60)); + + const snapshot = JSON.parse(mod.kicadCollabSetRemote.mock.calls.at(-1)![0]); + expect(snapshot.locks).toEqual([ + { uuid: "u1", name: "bob" }, + { uuid: "u2", name: "local" }, + ]); + }); + + it("releases contested holds when the local client loses the tiebreak", async () => { + const release = vi.fn(); + const mod = { ...fakeModule(), kicadCollabReleaseSelection: release }; + const win: PresenceKicadWindow = {}; + const presence = stubPresence(); // self = (local, 100) + bindKicadPresence({ mod, win, presence }); + + win.kicadCollab!.onSelection!('["contested","mine"]'); + // alice (wins: "alice" < "local") also holds "contested". + presence.fireClients([peer("alice", { clientId: 3, selection: ["contested"] })]); + await new Promise((r) => setTimeout(r, 60)); + + expect(release).toHaveBeenCalledWith('["contested"]', "alice"); + // The winner's snapshot keeps "contested" locked for us until we release. + const snapshot = JSON.parse(mod.kicadCollabSetRemote.mock.calls.at(-1)![0]); + expect(snapshot.locks).toEqual([{ uuid: "contested", name: "alice" }]); + }); + + it("does NOT release when the local client wins, and unlocks the won uuid", async () => { + const release = vi.fn(); + const mod = { ...fakeModule(), kicadCollabReleaseSelection: release }; + const win: PresenceKicadWindow = {}; + const presence = stubPresence(); // self = (local, 100) + bindKicadPresence({ mod, win, presence }); + + win.kicadCollab!.onSelection!('["contested"]'); + // zed loses ("local" < "zed") — we keep the item and it must not be + // locked against us while zed's release is in flight. + presence.fireClients([peer("zed", { clientId: 3, selection: ["contested"] })]); + await new Promise((r) => setTimeout(r, 60)); + + expect(release).not.toHaveBeenCalled(); + const snapshot = JSON.parse(mod.kicadCollabSetRemote.mock.calls.at(-1)![0]); + expect(snapshot.locks).toEqual([]); + }); +}); + describe("bindKicadPresence × crossApp", () => { it("forwards selection emits (uuids + fpPaths) into the cross-app room", () => { const mod = fakeModule(); diff --git a/web/standalone/src/wasm/collab/presence-kicad.ts b/web/standalone/src/wasm/collab/presence-kicad.ts index ee3ede5..7064c86 100644 --- a/web/standalone/src/wasm/collab/presence-kicad.ts +++ b/web/standalone/src/wasm/collab/presence-kicad.ts @@ -2,6 +2,7 @@ import { symbolUuidFromFootprintPath } from "@pcbjam/shared"; import { clog } from "./debug"; import type { PresenceHandle, PresencePeer } from "./presence"; import type { CrossAppHandle } from "./cross-app"; +import { contestedReleases, remoteLocks, type LockClient } from "./lock-tiebreak"; /** * Wire the C++ presence bridge (collab-presence 0002) to the awareness layer: @@ -28,6 +29,9 @@ export interface PresenceKicadModule { /** 0006 (pcbnew builds): `{uuids, fpPaths}` — uuids plus the selected * footprints' schematic paths. Absent on older wasm. */ kicadCollabGetSelectionFull?(): string; + /** 0007: tiebreak release — the local client lost an overlapping hold; + * the wasm side cancels an in-flight move and unselects exactly these. */ + kicadCollabReleaseSelection?(uuidsJson: string, holder: string): void; } export interface PresenceKicadWindow { @@ -122,12 +126,17 @@ export function bindKicadPresence(opts: { }): { destroy(): void } { const { mod, win, presence, crossApp } = opts; + // The local selection as last emitted by C++ — the tiebreak (0007) compares + // it against every other client's published selection. + let ownSelection: string[] = []; + // C++ → awareness ------------------------------------------------------------ win.kicadCollab = { ...win.kicadCollab, onSelection: (uuidsJson) => { const parsed = parseSelectionEmit(uuidsJson); if (!parsed) return; // malformed emit — keep the last published selection + ownSelection = parsed.uuids; presence.setSelection(parsed.uuids); crossApp?.setSelection(parsed.uuids, parsed.fpPaths); }, @@ -146,6 +155,7 @@ export function bindKicadPresence(opts: { (mod.kicadCollabGetSelectionFull?.() ?? mod.kicadCollabGetSelection()) || "[]", ); if (seed?.uuids.length) { + ownSelection = seed.uuids; presence.setSelection(seed.uuids); crossApp?.setSelection(seed.uuids, seed.fpPaths); } @@ -165,6 +175,7 @@ export function bindKicadPresence(opts: { selection: string[]; xsel?: string[]; }>; + locks?: Array<{ uuid: string; name: string }>; } = { peers: peers.map((p: PresencePeer) => ({ id: p.user.id, @@ -174,6 +185,22 @@ export function bindKicadPresence(opts: { selection: p.selection, })), }; + // Soft-locks (0007): every OTHER client's held uuids (own user's other + // tabs included — presence.clients(), not the user-deduped peers()), + // minus what we hold and win. Losing overlaps trigger a release. + const self = { ...presence.self(), selection: ownSelection }; + const lockClients: LockClient[] = presence.clients().map((c) => ({ + userId: c.user.id, + clientId: c.clientId, + name: c.user.name, + selection: c.selection, + })); + snapshot.locks = remoteLocks(self, lockClients); + const release = contestedReleases(self, lockClients); + if (release && mod.kicadCollabReleaseSelection) { + clog("presence-kicad: lost selection tiebreak to", release.holder, "—", release.uuids); + mod.kicadCollabReleaseSelection(JSON.stringify(release.uuids), release.holder); + } // Cross-app peers (0006): rendered as ghost outlines on the mapped items. // One entry per awareness CLIENT (own other tabs included — that's the // single-user cross-probe), tagged with the source editor. diff --git a/web/standalone/src/wasm/collab/presence.test.ts b/web/standalone/src/wasm/collab/presence.test.ts index 5b9963c..2eea3ae 100644 --- a/web/standalone/src/wasm/collab/presence.test.ts +++ b/web/standalone/src/wasm/collab/presence.test.ts @@ -48,10 +48,14 @@ function client(channel: string): Client { return c; } -afterEach(() => { +afterEach(async () => { for (const c of clients) c.destroy(); clients = []; resetPresenceColorClaims(); + // Drain already-queued BroadcastChannel deliveries before vitest recycles + // the module registry — a late message otherwise trips the vite-node module + // proxy (flaky unhandled RangeError at the schema getter). + await new Promise((r) => setTimeout(r, 20)); }); describe("presence over the BroadcastChannel awareness relay", () => { @@ -101,6 +105,14 @@ describe("presence over the BroadcastChannel awareness relay", () => { // bob sees ONE alice (two tabs, one person); alice's tabs don't see each other. expect(b.presence.peers().map((p) => p.user.id)).toEqual(["alice"]); expect(a1.presence.peers().map((p) => p.user.id)).toEqual(["bob"]); + + // clients() (0007 soft-locks) is per-CLIENT: no user dedupe, own-user + // other tabs included — alice's first tab sees her second tab AND bob. + expect(a1.presence!.clients().map((p) => p.user.id).sort()).toEqual(["alice", "bob"]); + expect(a1.presence!.self()).toEqual({ + userId: "alice", + clientId: a1.awareness.clientID, + }); }); it("drops malformed peer states instead of crashing the roster", async () => { diff --git a/web/standalone/src/wasm/collab/presence.ts b/web/standalone/src/wasm/collab/presence.ts index 6bb2080..0091757 100644 --- a/web/standalone/src/wasm/collab/presence.ts +++ b/web/standalone/src/wasm/collab/presence.ts @@ -31,6 +31,14 @@ export interface PresenceHandle { * user in two tabs is one person — keep the freshest state). */ peers(): PresencePeer[]; + /** + * Every OTHER awareness client in the room, validated but NOT deduped by + * user — the same user's other tab is its own entry. Soft-locks (0007) key + * on clients, not users: your own second tab must lock against you too. + */ + clients(): PresencePeer[]; + /** This client's identity for tiebreaks: (user.id, awareness clientID). */ + self(): { userId: string; clientId: number }; /** Fires with the new roster on every awareness change. Returns unsubscribe. */ subscribe(cb: (peers: PresencePeer[]) => void): () => void; /** 0002: publish the local pointer's world position (null = off canvas). */ @@ -167,7 +175,24 @@ export function createPresence(opts: { // holds our color, we yield and re-claim the lowest free slot — exactly one // side of any collision yields, so this converges without coordination. // Same-user tabs converge the other way: adopt the lower clientID's color. + // + // Re-entrancy guard: patch() fires the awareness 'change' event + // SYNCHRONOUSLY on the local instance, which re-enters this resolver — with + // two stale conflicting states in view (e.g. mid-propagation same-user + // tabs) the adopt/re-claim branches can ping-pong until the stack blows. + // Patches made HERE don't need immediate re-resolution; the next genuine + // (async) awareness delivery re-runs the resolver with fresher states. + let resolvingCollision = false; const resolveCollision = () => { + if (resolvingCollision) return; + resolvingCollision = true; + try { + resolveCollisionOnce(); + } finally { + resolvingCollision = false; + } + }; + const resolveCollisionOnce = () => { for (const [clientId, raw] of awareness.getStates()) { if (clientId === awareness.clientID) continue; const parsed = presenceStateSchema.safeParse(raw); @@ -194,13 +219,20 @@ export function createPresence(opts: { } }; - function peers(): PresencePeer[] { - const byUser = new Map(); + function clients(): PresencePeer[] { + const out: PresencePeer[] = []; for (const [clientId, raw] of awareness.getStates()) { if (clientId === awareness.clientID) continue; const parsed = presenceStateSchema.safeParse(raw); if (!parsed.success) continue; - const peer: PresencePeer = { ...parsed.data, clientId }; + out.push({ ...parsed.data, clientId }); + } + return out.sort((a, b) => a.clientId - b.clientId); + } + + function peers(): PresencePeer[] { + const byUser = new Map(); + for (const peer of clients()) { // Another tab of the SAME user isn't a peer — the roster shows other people. if (peer.user.id === user.id) continue; const existing = byUser.get(peer.user.id); @@ -231,6 +263,10 @@ export function createPresence(opts: { let destroyed = false; return { peers, + clients, + self() { + return { userId: user.id, clientId: awareness.clientID }; + }, subscribe(cb) { subscribers.add(cb); return () => subscribers.delete(cb);