diff --git a/excalidraw-app/app_constants.ts b/excalidraw-app/app_constants.ts index e115fd79..5caca9ff 100644 --- a/excalidraw-app/app_constants.ts +++ b/excalidraw-app/app_constants.ts @@ -54,6 +54,7 @@ export const STORAGE_KEYS = { // node id across reloads, which keeps previously shared room links dialable. // Transport identity only — the room capability remains the ticket itself. LOCAL_STORAGE_IROH_SECRET_KEY: "excalidraw-iroh-secret-key", + LOCAL_STORAGE_ANON_USER_ID: "excalidraw-anon-user-id", LOCAL_SCENE_ID: "excalidraw-local-scene-id", VERSION_DATA_STATE: "version-dataState", VERSION_FILES: "version-files", diff --git a/excalidraw-app/collab/Collab.tsx b/excalidraw-app/collab/Collab.tsx index 3b592bd0..3dc7d00c 100644 --- a/excalidraw-app/collab/Collab.tsx +++ b/excalidraw-app/collab/Collab.tsx @@ -1044,7 +1044,8 @@ class Collab extends PureComponent { break; } case WS_SUBTYPES.MOUSE_LOCATION: { - const { pointer, button, username, selectedElementIds } = data.payload; + const { pointer, button, username, selectedElementIds, userId } = + data.payload; const socketId: SocketUpdateDataSource["MOUSE_LOCATION"]["payload"]["socketId"] = data.payload.socketId || @@ -1056,6 +1057,7 @@ class Collab extends PureComponent { button, selectedElementIds, username, + ...(userId ? { id: userId } : {}), }); break; @@ -1091,10 +1093,11 @@ class Collab extends PureComponent { } case WS_SUBTYPES.IDLE_STATUS: { - const { userState, socketId, username } = data.payload; + const { userState, socketId, username, userId } = data.payload; this.updateCollaborator(socketId, { userState, username, + ...(userId ? { id: userId } : {}), }); break; } diff --git a/excalidraw-app/collab/Portal.ts b/excalidraw-app/collab/Portal.ts index 7f7529f9..194cba23 100644 --- a/excalidraw-app/collab/Portal.ts +++ b/excalidraw-app/collab/Portal.ts @@ -9,6 +9,7 @@ import { WS_SUBTYPES } from "../app_constants"; import { isSyncableElement } from "../data/collab"; import { IrohClient } from "./irohClient"; +import { getStableUserId } from "./userIdentity"; import type { SocketUpdateData, @@ -275,6 +276,7 @@ class Portal { type: WS_SUBTYPES.IDLE_STATUS, payload: { socketId: this.nodeId, + userId: getStableUserId(), userState, username: this.collab.state.username, }, @@ -295,6 +297,7 @@ class Portal { type: WS_SUBTYPES.MOUSE_LOCATION, payload: { socketId: this.nodeId, + userId: getStableUserId(), pointer: payload.pointer, button: payload.button || "up", selectedElementIds: diff --git a/excalidraw-app/collab/userIdentity.ts b/excalidraw-app/collab/userIdentity.ts new file mode 100644 index 00000000..afc93bcc --- /dev/null +++ b/excalidraw-app/collab/userIdentity.ts @@ -0,0 +1,57 @@ +/** + * User identity: who the person at the keyboard is, as opposed to which + * endpoint they happen to be talking through. Unlike the iroh secret key + * (transport identity, one per browser profile and re-minted on retries), this + * id is shared across every tab and every node id a browser produces, so the + * roster can collapse all of them into a single avatar. + */ + +import { randomId } from "@excalidraw/common"; + +import { appJotaiStore } from "../app-jotai"; +import { STORAGE_KEYS } from "../app_constants"; +import { atprotoAuthAtom } from "../data/atproto/auth"; + +let cachedAnonymousUserId: string | null = null; + +const readAnonymousUserId = (): string | null => { + try { + return localStorage.getItem(STORAGE_KEYS.LOCAL_STORAGE_ANON_USER_ID); + } catch (error: any) { + // Unable to access localStorage + console.error(error); + return null; + } +}; + +const writeAnonymousUserId = (id: string) => { + try { + localStorage.setItem(STORAGE_KEYS.LOCAL_STORAGE_ANON_USER_ID, id); + } catch (error: any) { + // Unable to access localStorage + console.error(error); + } +}; + +export const getOrCreateAnonymousUserId = (): string => { + if (cachedAnonymousUserId) { + return cachedAnonymousUserId; + } + const existing = readAnonymousUserId(); + if (existing) { + cachedAnonymousUserId = existing; + return existing; + } + const id = randomId(); + writeAnonymousUserId(id); + cachedAnonymousUserId = id; + return id; +}; + +/** + * Prefers the signed-in atproto did; falls back to the anonymous local id. Read + * lazily off the store — never awaits auth init, so this stays safe to call + * from the hot broadcast paths. + */ +export const getStableUserId = (): string => + appJotaiStore.get(atprotoAuthAtom)?.did ?? getOrCreateAnonymousUserId(); diff --git a/excalidraw-app/data/collab.ts b/excalidraw-app/data/collab.ts index baabd5b9..065a1c43 100644 --- a/excalidraw-app/data/collab.ts +++ b/excalidraw-app/data/collab.ts @@ -75,6 +75,7 @@ export type SocketUpdateDataSource = { type: WS_SUBTYPES.MOUSE_LOCATION; payload: { socketId: SocketId; + userId?: string; pointer: { x: number; y: number; tool: "pointer" | "laser" }; button: "down" | "up"; selectedElementIds: AppState["selectedElementIds"]; @@ -93,6 +94,7 @@ export type SocketUpdateDataSource = { type: WS_SUBTYPES.IDLE_STATUS; payload: { socketId: SocketId; + userId?: string; userState: UserIdleState; username: string; }; diff --git a/excalidraw-app/tests/collab.test.tsx b/excalidraw-app/tests/collab.test.tsx index 98070bf8..58231738 100644 --- a/excalidraw-app/tests/collab.test.tsx +++ b/excalidraw-app/tests/collab.test.tsx @@ -247,10 +247,15 @@ const fakeFile = (id: string) => ({ }); /** an IDLE_STATUS message as a peer's presence ping puts it on the wire */ -const presencePing = (socketId: string, username: string) => +const presencePing = (socketId: string, username: string, userId?: string) => ({ type: "IDLE_STATUS", - payload: { socketId, userState: "active", username }, + payload: { + socketId, + userState: "active", + username, + ...(userId ? { userId } : {}), + }, } as any); const decode = (payload: Uint8Array) => @@ -1313,6 +1318,14 @@ describe("collaboration", () => { expect.objectContaining({ username: "pinger", userState: "active" }), ); + // ...along with the stable user id that lets a second tab of ours + // collapse into a single avatar (no atproto session here, so it's the + // anonymous localStorage id) + const joinUserId = presence.at(-1)?.payload.userId; + expect(typeof joinUserId).toBe("string"); + expect(joinUserId).not.toBe(""); + expect(joinUserId).toBe(localStorage.getItem("excalidraw-anon-user-id")); + // ...and we keep saying so on the heartbeat presence.length = 0; await act(async () => { @@ -1320,6 +1333,7 @@ describe("collaboration", () => { }); expect(presence.length).toBeGreaterThan(0); expect(presence[0].payload.username).toBe("pinger"); + expect(presence[0].payload.userId).toBe(joinUserId); peerRoom.close(); await act(async () => { @@ -1330,6 +1344,133 @@ describe("collaboration", () => { } }); + it("stamps a peer's stable userId onto the roster entry", async () => { + window.history.replaceState({}, "", "/"); + await render(); + + await act(async () => { + await window.collab.startCollaboration(null); + }); + await waitFor(() => { + expect(window.collab.isCollaborating()).toBe(true); + }); + + // an IDLE_STATUS ping carrying the id + act(() => { + window.collab.onSocketMessage( + "idler" as any, + presencePing("idler", "Idler", "user-alpha"), + ); + }); + await waitFor(() => { + expect(h.state.collaborators.get("idler" as any)?.id).toBe("user-alpha"); + }); + + // ...and the same via a MOUSE_LOCATION one + act(() => { + window.collab.onSocketMessage( + "mover" as any, + { + type: "MOUSE_LOCATION", + payload: { + socketId: "mover", + userId: "user-beta", + username: "Mover", + pointer: { x: 1, y: 2, tool: "pointer" }, + button: "up", + selectedElementIds: {}, + }, + } as any, + ); + }); + await waitFor(() => { + expect(h.state.collaborators.get("mover" as any)?.id).toBe("user-beta"); + }); + + await act(async () => { + window.collab.stopCollaboration(false); + }); + }); + + it("gives two sockets of the same user the same collaborator id", async () => { + window.history.replaceState({}, "", "/"); + await render(); + + await act(async () => { + await window.collab.startCollaboration(null); + }); + await waitFor(() => { + expect(window.collab.isCollaborating()).toBe(true); + }); + + // one person, two tabs: two node ids, one user id + act(() => { + window.collab.onSocketMessage( + "tab-1" as any, + presencePing("tab-1", "Twin", "user-twin"), + ); + window.collab.onSocketMessage( + "tab-2" as any, + presencePing("tab-2", "Twin", "user-twin"), + ); + }); + + // the roster still tracks both connections (the avatar collapse happens + // upstream in UserList, keyed off this id) + await waitFor(() => { + expect(h.state.collaborators.get("tab-1" as any)?.id).toBe("user-twin"); + expect(h.state.collaborators.get("tab-2" as any)?.id).toBe("user-twin"); + }); + + await act(async () => { + window.collab.stopCollaboration(false); + }); + }); + + it("keeps a learned userId when a legacy ping omits it", async () => { + window.history.replaceState({}, "", "/"); + await render(); + + await act(async () => { + await window.collab.startCollaboration(null); + }); + await waitFor(() => { + expect(window.collab.isCollaborating()).toBe(true); + }); + + act(() => { + window.collab.onSocketMessage( + "old-client" as any, + presencePing("old-client", "Old", "user-gamma"), + ); + }); + await waitFor(() => { + expect(h.state.collaborators.get("old-client" as any)?.id).toBe( + "user-gamma", + ); + }); + + // a payload from before this field existed must not blank the id out + act(() => { + window.collab.onSocketMessage( + "old-client" as any, + presencePing("old-client", "Old"), + ); + }); + await waitFor(() => { + expect(h.state.collaborators.get("old-client" as any)?.username).toBe( + "Old", + ); + }); + expect(h.state.collaborators.get("old-client" as any)?.id).toBe( + "user-gamma", + ); + + await act(async () => { + window.collab.stopCollaboration(false); + }); + }); + it("puts an unknown sender on the roster and evicts it once it goes silent", async () => { window.history.replaceState({}, "", "/"); await render(); diff --git a/excalidraw-app/tests/userIdentity.test.ts b/excalidraw-app/tests/userIdentity.test.ts new file mode 100644 index 00000000..fdde3803 --- /dev/null +++ b/excalidraw-app/tests/userIdentity.test.ts @@ -0,0 +1,19 @@ +import { describe, expect, it } from "vitest"; + +import { STORAGE_KEYS } from "../app_constants"; +import { getOrCreateAnonymousUserId } from "../collab/userIdentity"; + +describe("getOrCreateAnonymousUserId", () => { + it("persists and returns the same value on a second call", () => { + const first = getOrCreateAnonymousUserId(); + + expect(typeof first).toBe("string"); + expect(first).not.toBe(""); + expect(localStorage.getItem(STORAGE_KEYS.LOCAL_STORAGE_ANON_USER_ID)).toBe( + first, + ); + + const second = getOrCreateAnonymousUserId(); + expect(second).toBe(first); + }); +});