diff --git a/apps/realtime/src/handlers/connection.ts b/apps/realtime/src/handlers/connection.ts index aa134d8a771..bd6d143aa36 100644 --- a/apps/realtime/src/handlers/connection.ts +++ b/apps/realtime/src/handlers/connection.ts @@ -1,5 +1,5 @@ import { createLogger } from '@sim/logger' -import { parseRoomName, type RoomRef, roomName } from '@sim/realtime-protocol/rooms' +import { parseRoomName, ROOM_TYPES, type RoomRef, roomName } from '@sim/realtime-protocol/rooms' import { cleanupFileDocForSocket } from '@/handlers/file-doc' import { cleanupPendingSubblocksForSocket } from '@/handlers/subblocks' import { cleanupPendingVariablesForSocket } from '@/handlers/variables' @@ -8,6 +8,14 @@ import type { IRoomManager } from '@/rooms' const logger = createLogger('ConnectionHandlers') +/** + * Room types whose presence lives in the room manager (Redis-backed), so a disconnect must + * remove the socket + broadcast a correction. The workspace-files and file-doc rooms are + * NOT here: workspace-files carries no presence (native Socket.IO membership only), and + * file-doc broadcasts its own server-authenticated roster via `cleanupFileDocForSocket`. + */ +const PRESENCE_BEARING_TYPES = new Set([ROOM_TYPES.WORKFLOW, ROOM_TYPES.TABLE]) + export function setupConnectionHandlers(socket: AuthenticatedSocket, roomManager: IRoomManager) { socket.on('error', (error) => { logger.error(`Socket ${socket.id} error:`, error) @@ -33,8 +41,9 @@ export function setupConnectionHandlers(socket: AuthenticatedSocket, roomManager cleanupPendingSubblocksForSocket(socket.id) cleanupPendingVariablesForSocket(socket.id) // Clear the socket's collaborative-document awareness (removes its caret for - // everyone else) and drop the room if it was the last editor. - cleanupFileDocForSocket(socket.id, roomManager.io) + // everyone else) and drop the room if it was the last editor. `endOfLife` drops the + // socket's join-generation entry — safe only here, on true disconnect (see cleanup). + cleanupFileDocForSocket(socket.id, roomManager.io, true) // A socket may occupy multiple rooms (one per type). Remove it from every // room the manager knows about. @@ -47,12 +56,13 @@ export function setupConnectionHandlers(socket: AuthenticatedSocket, roomManager const wasInRooms = new Map() for (const room of removedRooms) wasInRooms.set(roomName(room), room) for (const name of liveRoomNames) { - // `wasInRooms.has(name)` already excludes every room the manager removed - // (same room-name key via the roomName/parseRoomName bijection), so any - // room reaching here was NOT in `removedRooms` and needs a removal attempt. + // `wasInRooms.has(name)` already excludes every room the manager removed (same + // room-name key via the roomName/parseRoomName bijection). Skip room types with no + // manager-tracked presence (workspace-files, file-doc): removing there is a no-op and + // broadcasting a correction would emit a dead presence-update no client listens to. if (name === socket.id || wasInRooms.has(name)) continue const ref = parseRoomName(name) - if (!ref) continue + if (!ref || !PRESENCE_BEARING_TYPES.has(ref.type)) continue wasInRooms.set(name, ref) await roomManager.removeUserFromRoom(ref, socket.id) } diff --git a/apps/realtime/src/handlers/file-doc.test.ts b/apps/realtime/src/handlers/file-doc.test.ts index 48da76f22ac..1fbecde9078 100644 --- a/apps/realtime/src/handlers/file-doc.test.ts +++ b/apps/realtime/src/handlers/file-doc.test.ts @@ -54,6 +54,8 @@ function createSocket(id: string, overrides?: Record) { id, userId: 'user-1', userName: 'Test User', + // Set so the server's roster resolves the avatar from the socket (never the DB). + userImage: 'avatar.png', disconnected: false, on: vi.fn((event: string, handler: Handler) => { handlers[event] = handler @@ -133,7 +135,9 @@ describe('setupWorkspaceFileDocHandlers', () => { afterEach(() => { // The room store is module-global; drop every room the test's sockets opened. const { io } = createIo() - for (const id of createdSocketIds) cleanupFileDocForSocket(id, io) + // Simulate a full disconnect between tests (`endOfLife`) so the module-global join-generation + // map is cleared and never bleeds a counter into the next test. + for (const id of createdSocketIds) cleanupFileDocForSocket(id, io, true) createdSocketIds.clear() vi.clearAllTimers() vi.useRealTimers() @@ -435,7 +439,7 @@ describe('setupWorkspaceFileDocHandlers', () => { const pending = s.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 }) s.socket.disconnected = true - cleanupFileDocForSocket('socket-a', io) // disconnect cleanup — no-op, nothing registered yet + cleanupFileDocForSocket('socket-a', io, true) // disconnect cleanup — no-op, nothing registered yet resolveAuth({ allowed: true, status: 200, workspacePermission: 'write' }) await pending @@ -462,6 +466,49 @@ describe('setupWorkspaceFileDocHandlers', () => { expect(s.socket.join).toHaveBeenCalledWith('workspace-file-doc:file-2') }) + it('does not reset the join generation on a leave, so an in-flight join still binds', async () => { + const { io } = createIo() + const s = setup('socket-a', io) + + // file-1 join completes; the socket is registered in file-1. + await s.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 }) + + // file-2 join goes in-flight (authorize deferred). + let resolveAuth: (v: unknown) => void = () => {} + mockAuthorizeRoom.mockReturnValueOnce(new Promise((resolve) => (resolveAuth = resolve))) + const pending = s.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-2', clientId: 1 }) + + // A deferred leave for the prior file-1 lands while file-2's join awaits authorization. Its + // cleanup must NOT reset the monotonic join generation, or file-2's guard would see an emptied + // map (`undefined !== generation`) and abort the join the client actually wants. + s.handlers[FILE_DOC_EVENTS.LEAVE]({ fileId: 'file-1' }) + + resolveAuth({ allowed: true, status: 200, workspacePermission: 'write' }) + await pending + + expect(joinSuccessFileId(s.socket)).toBe('file-2') + expect(s.socket.join).toHaveBeenCalledWith('workspace-file-doc:file-2') + }) + + it('cancels an in-flight join when the client leaves that same file (no ghost owner)', async () => { + const { io, sent } = createIo() + let resolveAuth: (v: unknown) => void = () => {} + mockAuthorizeRoom.mockReturnValueOnce(new Promise((resolve) => (resolveAuth = resolve))) + const s = setup('socket-a', io) + + // Join file-1 is awaiting authorization when the client leaves file-1 (fast open→close). + const pending = s.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 }) + s.handlers[FILE_DOC_EVENTS.LEAVE]({ fileId: 'file-1' }) + resolveAuth({ allowed: true, status: 200, workspacePermission: 'write' }) + await pending + + // The stale join must not register: no success, no room join, and no presence broadcast that + // would leave a ghost collaborator until disconnect. + expect(s.socket.join).not.toHaveBeenCalled() + expect(joinSuccessFileId(s.socket)).toBeUndefined() + expect(sent.some((m) => m.event === FILE_DOC_EVENTS.PRESENCE)).toBe(false) + }) + it('scopes LEAVE to the named file (a leave for a different file is a no-op)', async () => { const { io } = createIo() const a = setup('socket-a', io) @@ -574,4 +621,40 @@ describe('setupWorkspaceFileDocHandlers', () => { // No re-election: the successful seed cancelled the deadline. expect(sent.find((m) => m.event === FILE_DOC_EVENTS.SEED_REQUEST)).toBeUndefined() }) + + it('broadcasts a server-authenticated presence roster on join, one entry per session', async () => { + const { io, sent } = createIo() + const a = setup('socket-a', io, { userId: 'user-a', userName: 'Ada', userImage: 'ada.png' }) + const b = setup('socket-b', io, { userId: 'user-b', userName: 'Bob', userImage: 'bob.png' }) + + await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 }) + await b.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 2 }) + + const roster = sent.filter((m) => m.event === FILE_DOC_EVENTS.PRESENCE).at(-1)?.payload as { + fileId: string + users: Array<{ socketId: string; userId: string; userName: string; avatarUrl: string | null }> + } + expect(roster.fileId).toBe('file-1') + // Identity is each socket's authenticated session — not any client-supplied value. + expect([...roster.users].sort((x, y) => x.userId.localeCompare(y.userId))).toEqual([ + { socketId: 'socket-a', userId: 'user-a', userName: 'Ada', avatarUrl: 'ada.png' }, + { socketId: 'socket-b', userId: 'user-b', userName: 'Bob', avatarUrl: 'bob.png' }, + ]) + }) + + it('keeps a per-session entry for two sockets of the SAME user (no server-side user dedup)', async () => { + const { io, sent } = createIo() + // Two tabs of one account: the client self-excludes its own socket, so the roster must carry + // BOTH sessions or a client could never see the other tab as present. + const a = setup('socket-a', io, { userId: 'user-a', userName: 'Ada', userImage: 'ada.png' }) + const b = setup('socket-b', io, { userId: 'user-a', userName: 'Ada', userImage: 'ada.png' }) + + await a.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 1 }) + await b.handlers[FILE_DOC_EVENTS.JOIN]({ fileId: 'file-1', clientId: 2 }) + + const roster = sent.filter((m) => m.event === FILE_DOC_EVENTS.PRESENCE).at(-1)?.payload as { + users: Array<{ socketId: string; userId: string }> + } + expect([...roster.users].map((u) => u.socketId).sort()).toEqual(['socket-a', 'socket-b']) + }) }) diff --git a/apps/realtime/src/handlers/file-doc.ts b/apps/realtime/src/handlers/file-doc.ts index a6c43c00cfa..d019a561bd8 100644 --- a/apps/realtime/src/handlers/file-doc.ts +++ b/apps/realtime/src/handlers/file-doc.ts @@ -4,6 +4,7 @@ import { FILE_DOC_EVENTS, FILE_DOC_MESSAGE_TYPE, FILE_DOC_SEED, + type FileDocPresenceUser, type JoinFileDocPayload, type LeaveFileDocPayload, toFileDocBytes, @@ -15,6 +16,7 @@ import type { Server } from 'socket.io' import * as awarenessProtocol from 'y-protocols/awareness' import * as syncProtocol from 'y-protocols/sync' import * as Y from 'yjs' +import { resolveAvatarUrl } from '@/handlers/avatar' import type { AuthenticatedSocket } from '@/middleware/auth' import type { IRoomManager } from '@/rooms' @@ -57,6 +59,10 @@ interface FileDocOwner { /** The owning user — used to tell a reconnect (same user reusing its Yjs client * id) from a spoof (a different user binding a peer's id). */ userId: string + /** Server-authenticated display identity for the presence roster (from the socket's + * session, never the client-set awareness — so a peer cannot spoof it). */ + userName: string + avatarUrl: string | null } interface FileDocRoom { @@ -116,6 +122,28 @@ function broadcast(io: Server, name: string, payload: Uint8Array, exceptSocketId channel.emit(FILE_DOC_EVENTS.MESSAGE, payload) } +/** + * Broadcast the room's collaborator roster to everyone in it, for the avatar stack. One entry + * PER SESSION (socket) — the client excludes its own socket and dedupes the remainder per user + * for display, so a second tab of the same account still registers as present (mirroring the + * canvas presence model). Deduping here instead would drop the current user's other sessions + * asymmetrically (only one socket survives), so each client could never reliably self-exclude. + * Identity comes from each owner's server-authenticated session — never the client-set awareness + * — so a peer cannot spoof or suppress an entry. + */ +function broadcastFileDocPresence(io: Server, name: string, room: FileDocRoom) { + const users: FileDocPresenceUser[] = [] + for (const [socketId, owner] of room.owners) { + users.push({ + socketId, + userId: owner.userId, + userName: owner.userName, + avatarUrl: owner.avatarUrl, + }) + } + io.to(name).emit(FILE_DOC_EVENTS.PRESENCE, { fileId: room.fileId, users }) +} + /** Whether the client has recorded that it seeded the document's initial content. */ function isDocSeeded(doc: Y.Doc): boolean { return doc.getMap(FILE_DOC_SEED.configMap).get(FILE_DOC_SEED.flag) === true @@ -316,10 +344,15 @@ function handleMessage(socket: AuthenticatedSocket, data: unknown) { * seeding, and drop the room's document when the last collaborator leaves. * Exported for the disconnect handler; safe to call for a socket in no room. */ -export function cleanupFileDocForSocket(socketId: string, io: Server): void { - // Drop the join generation so an in-flight JOIN for this socket aborts after - // its authorize resolves, and the map never leaks across the socket's life. - joinGeneration.delete(socketId) +export function cleanupFileDocForSocket(socketId: string, io: Server, endOfLife = false): void { + // The join-generation counter is monotonic for the socket's WHOLE life and must survive a room + // switch/leave: resetting it here would let the next join reuse a low number that a still + // in-flight earlier join also holds, so that stale join passes the generation guard and rebinds + // the socket to the wrong document. Drop it ONLY when the socket is truly gone (disconnect), + // which is also the only place the map would otherwise leak. An in-flight join is already + // aborted on disconnect by the `socket.disconnected` check, and on a switch by a newer join + // bumping the generation — neither needs this delete. + if (endOfLife) joinGeneration.delete(socketId) const name = socketToRoomName.get(socketId) if (!name) return @@ -334,6 +367,8 @@ export function cleanupFileDocForSocket(socketId: string, io: Server): void { // Fires the awareness `update` handler with a non-socket origin → the removal // is broadcast to every remaining client, so the departed caret vanishes. awarenessProtocol.removeAwarenessStates(room.awareness, [owner.clientId], null) + // Refresh the roster for whoever remains (server-authenticated identity). + broadcastFileDocPresence(io, name, room) } // Hand off the seeder role: if the elected seeder left before it seeded, elect @@ -351,12 +386,22 @@ export function cleanupFileDocForSocket(socketId: string, io: Server): void { * file id; joining requires workspace `write` (editing a document). Mirrors the * workspace-files join shape (auth → readiness → validate → authorize → join), * then runs the Yjs sync/awareness handshake. + * + * The avatar roster is derived from this room's own `owners` map and broadcast as + * `FILE_DOC_EVENTS.PRESENCE` — NOT the Redis-backed room-manager presence the workflow / + * table rooms use — because the file-doc room already owns an authoritative in-memory Y.Doc + * pinned to a single replica, so the session identity is right here with no extra store. */ export function setupWorkspaceFileDocHandlers( socket: AuthenticatedSocket, roomManager: IRoomManager ) { const io = roomManager.io + // The file this socket currently intends to edit (set when a join starts). A leave targeting it + // — or an unscoped leave — advances the join generation to cancel an in-flight join, so a join + // awaiting authorization can't complete after the client left and register a ghost owner. A + // leave for a DIFFERENT file must NOT cancel it (a document switch), mirroring workspace-files. + let currentFileId: string | null = null socket.on(FILE_DOC_EVENTS.JOIN, async ({ fileId, clientId }: JoinFileDocPayload) => { try { @@ -376,9 +421,11 @@ export function setupWorkspaceFileDocHandlers( return } - // Claim this JOIN's generation before the async authorize below. + // Claim this JOIN's generation before the async authorize below, and record the file the + // socket now intends to edit so a leave for it can cancel this join if it's still in-flight. const generation = (joinGeneration.get(socket.id) ?? 0) + 1 joinGeneration.set(socket.id, generation) + currentFileId = fileId const room = fileDocRoom(fileId) const name = roomName(room) @@ -408,9 +455,13 @@ export function setupWorkspaceFileDocHandlers( return } - // Abort a JOIN superseded during authorization: the socket disconnected, or - // a newer JOIN (a document switch) bumped the generation. Registering here - // would leak a dead socket's room or bind the socket to the wrong document. + // Server-authenticated identity for the presence roster (never trusts the client-set + // awareness). Resolved here so the generation guard below also covers this await. + const avatarUrl = await resolveAvatarUrl(socket, userId) + + // Abort a JOIN superseded during authorization/identity resolution: the socket + // disconnected, or a newer JOIN (a document switch) bumped the generation. Registering + // here would leak a dead socket's room or bind the socket to the wrong document. if (socket.disconnected || joinGeneration.get(socket.id) !== generation) return // Switched documents on the same socket — leave the previous one first (a @@ -449,11 +500,13 @@ export function setupWorkspaceFileDocHandlers( awarenessProtocol.removeAwarenessStates(entry.awareness, [previous.clientId], null) } - entry.owners.set(socket.id, { clientId, userId }) + entry.owners.set(socket.id, { clientId, userId, userName, avatarUrl }) socketToRoomName.set(socket.id, name) socket.join(name) socket.emit(FILE_DOC_EVENTS.JOIN_SUCCESS, { fileId }) + // Server-authenticated roster → everyone in the room, including this joiner. + broadcastFileDocPresence(io, name, entry) // Begin the sync handshake: send the server's state (sync step 1). The // client replies with its updates and requests the server's in return. @@ -496,10 +549,17 @@ export function setupWorkspaceFileDocHandlers( socket.on(FILE_DOC_EVENTS.LEAVE, (payload?: LeaveFileDocPayload) => { try { - // Only affect a REGISTERED room; never touch the join generation here. A - // leave that raced ahead of an in-flight join (no room registered yet) is a - // no-op — bumping the generation would silently abort an unrelated join for - // a different file (a document switch), leaving the socket bound to nothing. + // Cancel an in-flight join whose file the client is now leaving (or an unscoped leave): a + // join still awaiting authorization would otherwise complete after the client left, register + // as an owner, and broadcast a ghost collaborator until disconnect. Guard on the current + // file intent so a stale/deferred leave for a DIFFERENT file can't abort the join the client + // has since switched to (bumping the generation blindly caused that regression in #5941). + if (!payload?.fileId || payload.fileId === currentFileId) { + joinGeneration.set(socket.id, (joinGeneration.get(socket.id) ?? 0) + 1) + currentFileId = null + } + // Tear down membership only for a REGISTERED room; a leave that raced ahead of an in-flight + // join (nothing registered yet) has already cancelled it above. const name = socketToRoomName.get(socket.id) if (!name) return // Scope the leave to the named file when provided: a deferred leave from a diff --git a/apps/realtime/src/handlers/workspace-files.test.ts b/apps/realtime/src/handlers/workspace-files.test.ts index 76f20d9a7e5..350651c94a9 100644 --- a/apps/realtime/src/handlers/workspace-files.test.ts +++ b/apps/realtime/src/handlers/workspace-files.test.ts @@ -1,7 +1,6 @@ /** * @vitest-environment node */ -import { ROOM_TYPES } from '@sim/realtime-protocol/rooms' import { beforeEach, describe, expect, it, vi } from 'vitest' import type { IRoomManager } from '@/rooms' @@ -20,29 +19,28 @@ vi.mock('@sim/platform-authz/rooms', () => ({ import { setupWorkspaceFilesHandlers } from '@/handlers/workspace-files' -interface JoinPayload { - workspaceId: string - folderId?: string | null - tabSessionId?: string -} +type Payload = { workspaceId?: string } function createSocket(overrides?: Record) { - const handlers: Record Promise | void> = {} + const handlers: Record Promise | void> = {} + // Live Set so the handler's native `socket.rooms` membership tracking works in tests. + const rooms = new Set() const socket = { id: 'socket-1', userId: 'user-1', userName: 'Test User', userImage: 'avatar.png', - on: vi.fn((event: string, handler: (payload: JoinPayload) => Promise | void) => { + rooms, + on: vi.fn((event: string, handler: (payload?: Payload) => Promise | void) => { handlers[event] = handler }), emit: vi.fn(), - join: vi.fn(), - leave: vi.fn(), + join: vi.fn((room: string) => rooms.add(room)), + leave: vi.fn((room: string) => rooms.delete(room)), to: vi.fn().mockReturnValue({ emit: vi.fn() }), ...overrides, } - return { handlers, socket } + return { handlers, socket, rooms } } function createRoomManager(overrides?: Partial): IRoomManager { @@ -136,35 +134,101 @@ describe('setupWorkspaceFilesHandlers', () => { ) }) - it('joins the workspace files room and broadcasts presence on success', async () => { + it('joins the workspace files room on success without any presence bookkeeping', async () => { const { socket, handlers } = createSocket() - const roomManager = createRoomManager({ - getRoomUsers: vi.fn().mockResolvedValue([]), - }) + const roomManager = createRoomManager() setupWorkspaceFilesHandlers( socket as unknown as Parameters[0], roomManager ) - await handlers['join-workspace-files']({ + await handlers['join-workspace-files']({ workspaceId: 'ws-1' }) + + expect(socket.join).toHaveBeenCalledWith('workspace-files:ws-1') + expect(socket.emit).toHaveBeenCalledWith('join-workspace-files-success', { workspaceId: 'ws-1', - folderId: 'folder-1', - tabSessionId: 'tab-1', }) + // The room is live-tree-only: no room-manager presence is tracked or broadcast. + expect(roomManager.addUserToRoom).not.toHaveBeenCalled() + expect(roomManager.broadcastPresenceUpdate).not.toHaveBeenCalled() + }) + + it('leaves a previously-joined files room when switching workspaces', async () => { + const { socket, handlers, rooms } = createSocket() + rooms.add('workspace-files:ws-old') + setupWorkspaceFilesHandlers( + socket as unknown as Parameters[0], + createRoomManager() + ) + await handlers['join-workspace-files']({ workspaceId: 'ws-1' }) + + expect(socket.leave).toHaveBeenCalledWith('workspace-files:ws-old') expect(socket.join).toHaveBeenCalledWith('workspace-files:ws-1') - expect(roomManager.addUserToRoom).toHaveBeenCalledWith( - { type: ROOM_TYPES.WORKSPACE_FILES, id: 'ws-1' }, - 'socket-1', - expect.objectContaining({ userId: 'user-1', folderId: 'folder-1', role: 'admin' }) + }) + + it('leaves the scoped files room on leave', () => { + const { socket, handlers, rooms } = createSocket() + rooms.add('workspace-files:ws-1') + setupWorkspaceFilesHandlers( + socket as unknown as Parameters[0], + createRoomManager() ) - expect(socket.emit).toHaveBeenCalledWith( - 'join-workspace-files-success', - expect.objectContaining({ workspaceId: 'ws-1', socketId: 'socket-1' }) + + handlers['leave-workspace-files']({ workspaceId: 'ws-1' }) + + expect(socket.leave).toHaveBeenCalledWith('workspace-files:ws-1') + }) + + it('cancels an in-flight join when the user leaves that workspace mid-authorize', async () => { + const { socket, handlers } = createSocket() + let resolveAuth: (value: unknown) => void = () => {} + mockAuthorizeRoom.mockReturnValue( + new Promise((resolve) => { + resolveAuth = resolve + }) ) - expect(roomManager.broadcastPresenceUpdate).toHaveBeenCalledWith({ - type: ROOM_TYPES.WORKSPACE_FILES, - id: 'ws-1', + setupWorkspaceFilesHandlers( + socket as unknown as Parameters[0], + createRoomManager() + ) + + // Join ws-1 is awaiting authorization when the view unmounts and leaves ws-1. + const joinPromise = handlers['join-workspace-files']({ workspaceId: 'ws-1' }) + handlers['leave-workspace-files']({ workspaceId: 'ws-1' }) + resolveAuth({ allowed: true, status: 200, workspaceId: 'ws-1', workspacePermission: 'admin' }) + await joinPromise + + // The stale join must NOT join the room the client has since left (no stranded membership). + expect(socket.join).not.toHaveBeenCalled() + expect(socket.emit).not.toHaveBeenCalledWith('join-workspace-files-success', { + workspaceId: 'ws-1', + }) + }) + + it('does not cancel an in-flight join when a deferred leave targets a different workspace', async () => { + const { socket, handlers } = createSocket() + let resolveAuth: (value: unknown) => void = () => {} + mockAuthorizeRoom.mockReturnValue( + new Promise((resolve) => { + resolveAuth = resolve + }) + ) + setupWorkspaceFilesHandlers( + socket as unknown as Parameters[0], + createRoomManager() + ) + + // The client has switched to ws-2 (join in-flight) when a stale leave for the prior ws-1 lands. + const joinPromise = handlers['join-workspace-files']({ workspaceId: 'ws-2' }) + handlers['leave-workspace-files']({ workspaceId: 'ws-1' }) + resolveAuth({ allowed: true, status: 200, workspaceId: 'ws-2', workspacePermission: 'admin' }) + await joinPromise + + // The deferred leave for ws-1 must not abort the join the client actually wants (ws-2). + expect(socket.join).toHaveBeenCalledWith('workspace-files:ws-2') + expect(socket.emit).toHaveBeenCalledWith('join-workspace-files-success', { + workspaceId: 'ws-2', }) }) }) diff --git a/apps/realtime/src/handlers/workspace-files.ts b/apps/realtime/src/handlers/workspace-files.ts index 0c7d3ef8c9b..fa3b56111fa 100644 --- a/apps/realtime/src/handlers/workspace-files.ts +++ b/apps/realtime/src/handlers/workspace-files.ts @@ -1,10 +1,8 @@ import { createLogger } from '@sim/logger' import { authorizeRoom } from '@sim/platform-authz/rooms' import { ROOM_TYPES, type RoomRef, roomName } from '@sim/realtime-protocol/rooms' -import { resolveAvatarUrl } from '@/handlers/avatar' import type { AuthenticatedSocket } from '@/middleware/auth' -import type { IRoomManager, UserPresence } from '@/rooms' -import { filterVisiblePresence, sweepStalePresence } from '@/rooms/presence-visibility' +import type { IRoomManager } from '@/rooms' const logger = createLogger('WorkspaceFilesHandlers') @@ -14,188 +12,143 @@ const filesRoom = (workspaceId: string): RoomRef => ({ id: workspaceId, }) +/** Socket.IO room-name prefix shared by every workspace-files room. */ +const FILES_ROOM_PREFIX = `${ROOM_TYPES.WORKSPACE_FILES}:` + interface JoinPayload { workspaceId: string - folderId?: string | null - tabSessionId?: string } /** - * Presence handlers for the workspace file browser. Mirrors the workflow join - * flow but is workspace-scoped (room id = workspaceId) and read-only presence: - * there are no persisted file operations over the socket — file mutations go - * through the HTTP API, which fans out a `workspace-files-changed` event - * separately. The viewer's `folderId` is recorded at join (a future hook for - * folder-scoped presence); there is no cursor channel here yet. + * Keeps the workspace file browser live. The socket joins a workspace-scoped Socket.IO room + * so a `workspace-files-changed` event — fanned out by the HTTP mutation API — reaches every + * viewer, who then refetches. This room carries NO presence: "who's in a file" comes from + * the per-file doc room, and file mutations go over HTTP. Membership is tracked natively by + * Socket.IO (`socket.rooms`), so a workspace switch just leaves the prior files room — no + * room-manager presence bookkeeping to keep in sync. */ export function setupWorkspaceFilesHandlers( socket: AuthenticatedSocket, roomManager: IRoomManager ) { - socket.on( - 'join-workspace-files', - async ({ workspaceId, folderId, tabSessionId }: JoinPayload) => { - try { - const userId = socket.userId - const userName = socket.userName - - if (!userId || !userName) { - socket.emit('join-workspace-files-error', { - workspaceId, - error: 'Authentication required', - code: 'AUTHENTICATION_REQUIRED', - retryable: false, - }) - return - } - - if (!roomManager.isReady()) { - socket.emit('join-workspace-files-error', { - workspaceId, - error: 'Realtime unavailable', - code: 'ROOM_MANAGER_UNAVAILABLE', - retryable: true, - }) - return - } - - // Validate the client-supplied id before it reaches the DB query (matches - // the /api/workspace-files-changed guard; join payloads are otherwise raw - // client input). - if (typeof workspaceId !== 'string' || workspaceId.length === 0) { - socket.emit('join-workspace-files-error', { - workspaceId: typeof workspaceId === 'string' ? workspaceId : '', - error: 'Invalid workspace id', - code: 'INVALID_PAYLOAD', - retryable: false, - }) - return - } - - const room = filesRoom(workspaceId) - - let authorized: Awaited> - try { - authorized = await authorizeRoom({ userId, room, action: 'read' }) - } catch (error) { - logger.warn(`Error authorizing files room for ${userId}:`, error) - socket.emit('join-workspace-files-error', { - workspaceId, - error: 'Failed to verify workspace access', - code: 'VERIFY_ACCESS_FAILED', - retryable: true, - }) - return - } - - if (!authorized.allowed) { - socket.emit('join-workspace-files-error', { - workspaceId, - error: authorized.status === 404 ? 'Workspace not found' : 'Access denied to workspace', - code: authorized.status === 404 ? 'NOT_FOUND' : 'ACCESS_DENIED', - retryable: false, - }) - return - } - - // Leave a previously-joined files room if switching workspaces. - const currentRoom = await roomManager.getRoomForSocket( - socket.id, - ROOM_TYPES.WORKSPACE_FILES - ) - if (currentRoom && currentRoom.id !== workspaceId) { - socket.leave(roomName(currentRoom)) - await roomManager.removeUserFromRoom(currentRoom, socket.id) - await roomManager.broadcastPresenceUpdate(currentRoom) - } - - // Clean up the same user's stale socket from the same tab (e.g. a reconnect - // that raced the old socket's disconnect), so presence shows one entry. - if (tabSessionId) { - const existingUsers = await roomManager.getRoomUsers(room) - for (const existing of existingUsers) { - if ( - existing.socketId !== socket.id && - existing.userId === userId && - existing.tabSessionId === tabSessionId - ) { - await roomManager.removeUserFromRoom(room, existing.socketId) - await roomManager.io.in(existing.socketId).socketsLeave(roomName(room)) - } - } - } - - // Reclaim any presence orphaned by an ungraceful disconnect (pod crash - // fires no `disconnecting` event; the room hashes have no TTL). - await sweepStalePresence(roomManager, room) - - socket.join(roomName(room)) - - const presence: UserPresence = { - userId, - room, - userName, - socketId: socket.id, - tabSessionId, - joinedAt: Date.now(), - lastActivity: Date.now(), - role: authorized.workspacePermission ?? 'read', - folderId: folderId ?? null, - avatarUrl: await resolveAvatarUrl(socket, userId), - } - - await roomManager.addUserToRoom(room, socket.id, presence) + // Monotonic per-socket join counter: each join captures its number and, after the async + // authorize, aborts if a newer intent has superseded it — a fast workspace switch A→B can + // otherwise let A's late completion leave B and strand the socket in A, missing B's + // `workspace-files-changed` invalidations. + let joinGeneration = 0 + // The workspace the socket currently intends to be in (set when a join starts). A leave that + // targets this workspace — or an unscoped "leave all" — advances joinGeneration so an in-flight + // join is cancelled instead of completing after the view has closed. A stale/deferred leave for + // a DIFFERENT workspace must NOT advance it, or it would abort the join the client has since + // switched to (the bug that bit the file-doc room in #5941). + let currentWorkspace: string | null = null + + socket.on('join-workspace-files', async ({ workspaceId }: JoinPayload) => { + const joinAttempt = (joinGeneration += 1) + currentWorkspace = workspaceId + try { + if (!socket.userId || !socket.userName) { + socket.emit('join-workspace-files-error', { + workspaceId, + error: 'Authentication required', + code: 'AUTHENTICATION_REQUIRED', + retryable: false, + }) + return + } - // Filter the join ack to live members so a new joiner never briefly sees a - // ghost from an entry the sweep hasn't reclaimed yet. - const presenceUsers = await filterVisiblePresence( - roomManager.io, - room, - await roomManager.getRoomUsers(room) - ) - socket.emit('join-workspace-files-success', { + if (!roomManager.isReady()) { + socket.emit('join-workspace-files-error', { workspaceId, - socketId: socket.id, - presenceUsers, + error: 'Realtime unavailable', + code: 'ROOM_MANAGER_UNAVAILABLE', + retryable: true, }) + return + } + + // Validate the client-supplied id before it reaches the DB query (join payloads are + // otherwise raw client input). + if (typeof workspaceId !== 'string' || workspaceId.length === 0) { + socket.emit('join-workspace-files-error', { + workspaceId: typeof workspaceId === 'string' ? workspaceId : '', + error: 'Invalid workspace id', + code: 'INVALID_PAYLOAD', + retryable: false, + }) + return + } - await roomManager.broadcastPresenceUpdate(room) + const room = filesRoom(workspaceId) - logger.info(`User ${userId} (${userName}) joined files room for workspace ${workspaceId}`) + let authorized: Awaited> + try { + authorized = await authorizeRoom({ userId: socket.userId, room, action: 'read' }) } catch (error) { - logger.error('Error joining workspace files room:', error) - // Roll back any partial join so a failed attempt can't leave the socket in - // the Socket.IO room or a stale presence entry behind (mirrors the workflow - // join's rollback), before signalling a retryable failure. - try { - const room = filesRoom(workspaceId) - socket.leave(roomName(room)) - await roomManager.removeUserFromRoom(room, socket.id) - } catch {} + logger.warn(`Error authorizing files room for ${socket.userId}:`, error) socket.emit('join-workspace-files-error', { workspaceId, - error: 'Failed to join workspace files', - code: 'JOIN_FAILED', + error: 'Failed to verify workspace access', + code: 'VERIFY_ACCESS_FAILED', retryable: true, }) + return } - } - ) - socket.on('leave-workspace-files', async (payload?: { workspaceId?: string }) => { - try { - if (!roomManager.isReady()) return - const room = await roomManager.getRoomForSocket(socket.id, ROOM_TYPES.WORKSPACE_FILES) - if (!room) return - // Scope the leave to a specific workspace when the client provides one: a - // deferred leave from a prior page must not evict the socket from a room it - // has since switched into (workspace A→B leaves A's leave targeting B). - if (payload?.workspaceId && payload.workspaceId !== room.id) return - socket.leave(roomName(room)) - await roomManager.removeUserFromRoom(room, socket.id) - await roomManager.broadcastPresenceUpdate(room, socket.id) + if (!authorized.allowed) { + socket.emit('join-workspace-files-error', { + workspaceId, + error: authorized.status === 404 ? 'Workspace not found' : 'Access denied to workspace', + code: authorized.status === 404 ? 'NOT_FOUND' : 'ACCESS_DENIED', + retryable: false, + }) + return + } + + // A newer join started on this socket during authorize (or it dropped): abort so a + // stale join can't leave the room the client has since switched to. + if (joinGeneration !== joinAttempt || socket.disconnected) return + + // Leave any previously-joined files room (workspace switch), read straight from the + // socket's native room membership so there's no presence store to keep in sync. + const target = roomName(room) + for (const joined of socket.rooms) { + if (joined !== target && joined.startsWith(FILES_ROOM_PREFIX)) socket.leave(joined) + } + + socket.join(target) + socket.emit('join-workspace-files-success', { workspaceId }) } catch (error) { - logger.error('Error leaving workspace files room:', error) + logger.error('Error joining workspace files room:', error) + try { + socket.leave(roomName(filesRoom(workspaceId))) + } catch {} + socket.emit('join-workspace-files-error', { + workspaceId, + error: 'Failed to join workspace files', + code: 'JOIN_FAILED', + retryable: true, + }) + } + }) + + socket.on('leave-workspace-files', (payload?: { workspaceId?: string }) => { + // Cancel an in-flight join whose target the client is now leaving: a join awaiting + // authorization when the view unmounts would otherwise complete afterwards and strand the + // socket in a room it has left. Only when the leave targets the current join intent (or is + // unscoped) — a deferred leave for a different workspace must not abort the join the client + // has since switched to. + if (!payload?.workspaceId || payload.workspaceId === currentWorkspace) { + joinGeneration += 1 + currentWorkspace = null + } + // Scope the leave to a specific workspace when the client provides one: a deferred leave + // from a prior page must not evict a files room the socket has since switched into. + const target = payload?.workspaceId ? roomName(filesRoom(payload.workspaceId)) : null + for (const joined of socket.rooms) { + if (!joined.startsWith(FILES_ROOM_PREFIX)) continue + if (target && joined !== target) continue + socket.leave(joined) } }) } diff --git a/apps/realtime/src/index.test.ts b/apps/realtime/src/index.test.ts index 5015b58d9bf..eb7ee030937 100644 --- a/apps/realtime/src/index.test.ts +++ b/apps/realtime/src/index.test.ts @@ -4,10 +4,10 @@ * @vitest-environment node */ import { createServer, request as httpRequest } from 'http' +import { ROOM_TYPES } from '@sim/realtime-protocol/rooms' import { createMockLogger } from '@sim/testing' import { randomInt } from '@sim/utils/random' import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' -import { ROOM_TYPES } from '@sim/realtime-protocol/rooms' import { createSocketIOServer } from '@/config/socket' import { MemoryRoomManager, workflowRoom } from '@/rooms' import { createHttpHandler } from '@/routes/http' diff --git a/apps/sim/app/workspace/[workspaceId]/components/presence/presence-avatars.tsx b/apps/sim/app/workspace/[workspaceId]/components/presence/presence-avatars.tsx index 3698578f8af..70c75e416b1 100644 --- a/apps/sim/app/workspace/[workspaceId]/components/presence/presence-avatars.tsx +++ b/apps/sim/app/workspace/[workspaceId]/components/presence/presence-avatars.tsx @@ -6,7 +6,9 @@ import { getUserColor } from '@/lib/workspaces/colors' /** Minimal presence shape the avatar stack renders — shared by workflow and files. */ export interface PresenceAvatarUser { - socketId: string + /** Unique id per presence entry, used as the render key: a socket id where presence is + * per-connection (workflow), absent where entries are deduped per user (file docs). */ + socketId?: string userId: string userName?: string avatarUrl?: string | null @@ -107,7 +109,7 @@ export function PresenceAvatars({ users, maxVisible = DEFAULT_MAX_VISIBLE }: Pre )} {visibleUsers.map((user, index) => ( - + ))} ) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/caret-presence.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/caret-presence.test.ts new file mode 100644 index 00000000000..523335ad8e5 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/caret-presence.test.ts @@ -0,0 +1,55 @@ +/** + * @vitest-environment jsdom + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { activateCaretLabel, CARET_LABEL_HOLD_MS, renderCaret } from './caret-presence' + +const ACTIVE = 'collaboration-carets__caret--active' +const FLIP = 'collaboration-carets__caret--flip' + +describe('caret-presence', () => { + beforeEach(() => vi.useFakeTimers()) + afterEach(() => vi.useRealTimers()) + + it('builds a tagged caret with a name label, shown on appearance', () => { + const caret = renderCaret({ name: 'Ada', color: '#f783ac', clientId: 4242 }) + expect(caret.classList.contains('collaboration-carets__caret')).toBe(true) + expect(caret.dataset.caretClientId).toBe('4242') + expect(caret.style.getPropertyValue('--caret-color')).toBeTruthy() + const label = caret.querySelector('.collaboration-carets__label') + expect(label?.textContent).toBe('Ada') + expect(caret.classList.contains(ACTIVE)).toBe(true) + }) + + it('falls back to a default name for a bare user state', () => { + const caret = renderCaret({ clientId: 1 }) + expect(caret.querySelector('.collaboration-carets__label')?.textContent).toBe('Collaborator') + }) + + it('hides the label after the inactivity hold, and re-activation restarts it', () => { + const caret = renderCaret({ name: 'Ada', color: '#f783ac', clientId: 4242 }) + vi.advanceTimersByTime(CARET_LABEL_HOLD_MS - 1) + expect(caret.classList.contains(ACTIVE)).toBe(true) + vi.advanceTimersByTime(1) + expect(caret.classList.contains(ACTIVE)).toBe(false) + + activateCaretLabel(caret) + expect(caret.classList.contains(ACTIVE)).toBe(true) + vi.advanceTimersByTime(CARET_LABEL_HOLD_MS) + expect(caret.classList.contains(ACTIVE)).toBe(false) + }) + + it('flips the label left only when it would overflow the editor right edge', () => { + const caret = renderCaret({ name: 'Ada', color: '#f783ac', clientId: 4242 }) + const label = caret.querySelector('.collaboration-carets__label') + if (!label) throw new Error('label missing') + // double-cast-allowed: jsdom has no layout; stub the label's right edge for the measure + label.getBoundingClientRect = () => ({ right: 500 }) as unknown as DOMRect + + activateCaretLabel(caret, 600) // editor edge past the label → no flip + expect(caret.classList.contains(FLIP)).toBe(false) + + activateCaretLabel(caret, 400) // editor edge before the label's right → flip + expect(caret.classList.contains(FLIP)).toBe(true) + }) +}) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/caret-presence.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/caret-presence.ts new file mode 100644 index 00000000000..eff0ce70534 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/caret-presence.ts @@ -0,0 +1,143 @@ +import { Extension } from '@tiptap/core' +import { Plugin, PluginKey } from '@tiptap/pm/state' +import type { Awareness } from 'y-protocols/awareness' + +/** + * Remote-collaborator caret presence for the file editor: the name label's + * show-then-fade behavior and the edge-aware flip, plus the ProseMirror plugin + * that drives them off awareness activity. + * + * Matches Google Docs: the name flag shows only while the peer has typed in the last + * few seconds or on local hover, then hides after inactivity, leaving just the caret. + */ + +/** + * How long a peer's name label stays visible after their last activity (cursor + * move / edit) before it fades, leaving just the colored caret bar. + */ +export const CARET_LABEL_HOLD_MS = 2000 + +/** Fallback caret color when a peer's awareness carries no `color`. */ +export const DEFAULT_CARET_COLOR = '#000000' + +/** + * The active-state class {@link activateCaretLabel} toggles on the caret node to reveal the + * name label; CSS transitions it back to hidden when removed. (yCursorPlugin reuses the DOM + * node and never re-runs `render`, which is why this file drives the class itself.) + */ +const CARET_ACTIVE_CLASS = 'collaboration-carets__caret--active' + +/** The class that flips the label to the caret's left near the editor's right edge. */ +const CARET_FLIP_CLASS = 'collaboration-carets__caret--flip' + +/** Per-caret fade timers, keyed by the (reused) caret DOM node. */ +const caretFadeTimers = new WeakMap>() + +/** + * Show a peer's name label, (re)start its fade timer, and flip it to the caret's + * left when it would run off the editor's right edge. + * + * yCursorPlugin renders each caret as a keyed widget decoration, so ProseMirror + * REUSES the same DOM node as the caret moves (verified: the render function is + * not re-invoked on a position change) — a CSS animation therefore cannot restart + * on activity. Instead the activity signal is the awareness `change` event, which + * (unlike `update`) fires only on a real state change, not the 15s heartbeat, so + * an idle peer's label correctly stays hidden. + */ +export function activateCaretLabel(caret: HTMLElement, editorRight?: number) { + caret.classList.add(CARET_ACTIVE_CLASS) + const existing = caretFadeTimers.get(caret) + if (existing) clearTimeout(existing) + caretFadeTimers.set( + caret, + setTimeout(() => { + caret.classList.remove(CARET_ACTIVE_CLASS) + caretFadeTimers.delete(caret) + }, CARET_LABEL_HOLD_MS) + ) + + if (editorRight === undefined) return + const label = caret.querySelector('.collaboration-carets__label') + if (!label) return + // Measure the default (rightward) position, then flip left only if it overflows. + caret.classList.remove(CARET_FLIP_CLASS) + if (label.getBoundingClientRect().right > editorRight) { + caret.classList.add(CARET_FLIP_CLASS) + } +} + +/** + * Builds a remote peer's caret DOM: a colored bar plus a name label tagged with + * the peer's Yjs client id (so {@link createCaretActivityExtension} can find and + * re-activate the reused node on later awareness changes). Shown immediately on + * (re)appearance; the fade timer hides it after inactivity. Passed to + * CollaborationCaret as its `render` option, which only supplies `user` — so the + * client id rides along in the awareness `user` payload (each client stamps its own + * `doc.clientID`; see `use-file-doc-collaboration.ts`). + */ +export function renderCaret(user: Record): HTMLElement { + const color = typeof user.color === 'string' ? user.color : DEFAULT_CARET_COLOR + const name = typeof user.name === 'string' && user.name ? user.name : 'Collaborator' + const clientId = typeof user.clientId === 'number' ? user.clientId : undefined + const caret = document.createElement('span') + caret.className = 'collaboration-carets__caret' + // One inline var drives the caret bar, the dormant cap, and the name tag (all in CSS). + caret.style.setProperty('--caret-color', color) + if (clientId !== undefined) caret.dataset.caretClientId = String(clientId) + const label = document.createElement('div') + label.className = 'collaboration-carets__label' + label.textContent = name + caret.appendChild(label) + activateCaretLabel(caret) + return caret +} + +/** + * Drives the caret name-label show-then-fade off awareness activity. Because the + * caret DOM node is reused across moves (see {@link activateCaretLabel}), the + * `render` function alone can't reveal the label when a peer moves — this listens + * for awareness `change` events and re-activates the matching caret node. Deferred + * to the next frame so the node exists and is laid out (for the edge-flip measure). + */ +export function createCaretActivityExtension(awareness: Awareness): Extension { + return Extension.create({ + name: 'collaborationCaretActivity', + addProseMirrorPlugins() { + return [ + new Plugin({ + key: new PluginKey('collaborationCaretActivity'), + view: (editorView) => { + // Coalesce bursts of awareness changes into a single rAF: at most one forced + // layout read + one flush per frame, however many peers moved. Accumulate the + // changed client ids, then re-activate each matching (reused) caret node. + let raf = 0 + const pending = new Set() + const flush = () => { + raf = 0 + const editorRight = editorView.dom.getBoundingClientRect().right + for (const id of pending) { + const caret = editorView.dom.querySelector( + `.collaboration-carets__caret[data-caret-client-id="${id}"]` + ) + if (caret) activateCaretLabel(caret, editorRight) + } + pending.clear() + } + const onChange = ({ added, updated }: { added: number[]; updated: number[] }) => { + for (const id of added) pending.add(id) + for (const id of updated) pending.add(id) + if (pending.size > 0 && !raf) raf = requestAnimationFrame(flush) + } + awareness.on('change', onChange) + return { + destroy: () => { + awareness.off('change', onChange) + if (raf) cancelAnimationFrame(raf) + }, + } + }, + }), + ] + }, + }) +} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-avatars.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-avatars.tsx new file mode 100644 index 00000000000..13464e720e1 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-avatars.tsx @@ -0,0 +1,14 @@ +'use client' + +import { PresenceAvatars } from '@/app/workspace/[workspaceId]/components/presence/presence-avatars' +import { useFileDocOthers } from './file-doc-room-context' + +/** + * Avatar stack of the collaborators currently in the open file — the `useOthers` avatar + * stack, reading the room roster from {@link useFileDocOthers}. Renders nothing until + * someone else joins. Must sit inside a `FileDocRoomProvider`. + */ +export function FileDocAvatars() { + const others = useFileDocOthers() + return +} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts index 97fd5e13996..ee9ee61923a 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.test.ts @@ -165,6 +165,26 @@ describe('FileDocProvider', () => { expect(messages.some((m) => m[0] === FILE_DOC_MESSAGE_TYPE.AWARENESS)).toBe(true) }) + it('reseeds a cleared awareness so a reused instance can publish again', () => { + const { socket, emit } = createSocket(true) + const doc = new Y.Doc() + const awareness = new awarenessProtocol.Awareness(doc) + // Simulate a prior provider teardown having cleared the local state — after + // this, y-protocols' setLocalStateField is a permanent no-op, so the caret + // extension could never publish the local user/cursor on a reused instance. + awarenessProtocol.removeAwarenessStates(awareness, [doc.clientID], 'prior-destroy') + expect(awareness.getLocalState()).toBeNull() + + // Constructing a provider on the reused, cleared awareness must restore it. + new FileDocProvider(socket, 'file-1', doc, awareness) + expect(awareness.getLocalState()).not.toBeNull() + + emit.mockClear() + // The caret extension setting the user field must now actually publish. + awareness.setLocalStateField('user', { name: 'Ada', color: '#f783ac' }) + expect(emittedMessages(emit).some((m) => m[0] === FILE_DOC_MESSAGE_TYPE.AWARENESS)).toBe(true) + }) + it('does not forward awareness it applied from the server', () => { const { emit, fire } = createProvider(true) emit.mockClear() diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts index 5472ab31668..f0d24f9c23a 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-provider.ts @@ -69,6 +69,18 @@ export class FileDocProvider extends ObservableV2 { ) { super() + // Restore an empty local awareness state if it has been cleared. A fresh + // Awareness starts with `{}`, but a *reused* one whose local state was removed + // (a prior provider's `destroy()` clears it, and so does `Awareness.destroy()`) + // returns `null` here — and y-protocols' `setLocalStateField` is a no-op while + // the local state is `null`. The editor binds CollaborationCaret to this exact + // awareness for its whole life, so without this reseed a remount (e.g. React + // StrictMode's mount→unmount→mount, which re-runs the provider effect on the + // same instance) would leave the caret extension unable to ever publish the + // local user/cursor — remote peers would see no caret or selection, even though + // document sync (which does not depend on local awareness) keeps working. + if (awareness.getLocalState() === null) awareness.setLocalState({}) + socket.on(FILE_DOC_EVENTS.MESSAGE, this.handleMessage) socket.on(FILE_DOC_EVENTS.JOIN_SUCCESS, this.handleJoinSuccess) socket.on(FILE_DOC_EVENTS.JOIN_ERROR, this.handleJoinError) diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.tsx b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.tsx new file mode 100644 index 00000000000..2afc31ae7f1 --- /dev/null +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context.tsx @@ -0,0 +1,43 @@ +'use client' + +import { createContext, type ReactNode, useContext, useState } from 'react' +import type { PresenceAvatarUser } from '@/app/workspace/[workspaceId]/components/presence/presence-avatars' + +const EMPTY_OTHERS: PresenceAvatarUser[] = [] +const noop = () => {} + +// Split into two contexts on purpose: the roster (`others`) changes on every join/leave, +// but the setter is stable. The editor (which owns the awareness) only ever *reports* the +// roster, so it subscribes to the setter context — which never changes identity — and never +// re-renders when the roster does; only the header avatar stack subscribes to `others`. +const FileDocOthersContext = createContext(EMPTY_OTHERS) +const FileDocSetOthersContext = createContext<(users: PresenceAvatarUser[]) => void>(noop) + +/** + * Scopes "who's in this file" presence to the open document — the `RoomProvider` + + * `useOthers` pattern (Liveblocks / y-presence) adapted to our component tree. The editor + * owns the Yjs awareness but sits *below* the file-detail header that renders the avatar + * stack, so it publishes the SERVER-AUTHENTICATED roster into this context + * ({@link useReportFileDocOthers}) and the header reads it ({@link useFileDocOthers}). + * Presence is ephemeral and room-scoped, so it lives in this provider, not a global store. + */ +export function FileDocRoomProvider({ children }: { children: ReactNode }) { + const [others, setOthers] = useState(EMPTY_OTHERS) + return ( + + {children} + + ) +} + +/** The roster of collaborators currently in the open file, for an avatar stack. Empty + * outside a {@link FileDocRoomProvider}. */ +export function useFileDocOthers(): PresenceAvatarUser[] { + return useContext(FileDocOthersContext) +} + +/** Publishes the server roster into the room context (editor side). Returns a stable no-op + * outside a {@link FileDocRoomProvider}. */ +export function useReportFileDocOthers(): (users: PresenceAvatarUser[]) => void { + return useContext(FileDocSetOthersContext) +} diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts index 9fe069b56d0..475ea92fb2e 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/use-file-doc-collaboration.ts @@ -1,11 +1,13 @@ 'use client' import { useEffect, useMemo, useRef, useState } from 'react' +import { FILE_DOC_EVENTS, type FileDocPresence } from '@sim/realtime-protocol/file-doc' import { Awareness } from 'y-protocols/awareness' import * as Y from 'yjs' import { getUserColor } from '@/lib/workspaces/colors' import { useSocket } from '@/app/workspace/providers/socket-provider' import { FileDocProvider } from './file-doc-provider' +import { useReportFileDocOthers } from './file-doc-room-context' /** The live collaboration binding the editor wires into TipTap's Collaboration * (the {@link Y.Doc}) and CollaborationCaret (the awareness). */ @@ -20,8 +22,13 @@ export interface FileDocCollaboration { * provider is consumed for seeding events (`synced` / `seed-request`). */ provider: FileDocProvider | null - /** Local caret identity for CollaborationCaret: display name + assigned color. */ - user: { name: string; color: string } + /** + * The local caret identity published to awareness: `name`/`color` for CollaborationCaret, + * and `clientId` so the caret activity extension can tag each caret node (see + * caret-presence.ts). The avatar roster does NOT come from here — it's server-authenticated + * (see the PRESENCE subscription below) so a peer can't spoof identity via awareness. + */ + user: { name: string; color: string; clientId: number | undefined } } interface UseFileDocCollaborationParams { @@ -57,26 +64,42 @@ export function useFileDocCollaboration({ // round-trip-unsafe views never build a Yjs document they won't use. const docRef = useRef(null) const awarenessRef = useRef(null) + // Created ONCE and kept stable for the whole editor lifetime. The editor freezes these into + // its extension set at mount (`useEditor` fixes extensions at creation), so the instances the + // provider binds MUST stay byte-identical to what the editor holds — never destroyed-and- + // recreated mid-life. If a StrictMode dev remount destroyed them, the still-mounted editor + // would keep the dead doc while the provider synced a fresh one, and a joining peer would see a + // blank document. Teardown is therefore deferred to a REAL unmount only (see below). if (enabled && docRef.current === null) { docRef.current = new Y.Doc() awarenessRef.current = new Awareness(docRef.current) } - const [provider, setProvider] = useState(null) - - // Declared BEFORE the provider effect so, on unmount, React runs this cleanup - // AFTER the provider effect's cleanup (cleanups run in reverse declaration - // order) — the provider detaches from the doc/awareness before they're destroyed. + // Destroy the doc + awareness on a REAL unmount only. `Awareness` runs a setInterval to expire + // stale peers, so it MUST be destroyed or that timer leaks for every file ever opened. But a + // StrictMode dev remount fires this cleanup and then re-runs the setup synchronously after — so + // we SCHEDULE the teardown and the remount cancels it, keeping the frozen instances alive. A + // genuine unmount has no remount, so the scheduled teardown runs on the next tick. + const pendingTeardownRef = useRef | null>(null) useEffect(() => { + if (pendingTeardownRef.current !== null) { + clearTimeout(pendingTeardownRef.current) + pendingTeardownRef.current = null + } return () => { - awarenessRef.current?.destroy() - docRef.current?.destroy() + pendingTeardownRef.current = setTimeout(() => { + awarenessRef.current?.destroy() + docRef.current?.destroy() + }, 0) } }, []) + const [provider, setProvider] = useState(null) + useEffect(() => { if (!enabled || !socket) return - // Non-null: both refs are lazily set during render, before any effect runs. + // Non-null: both refs are set during render before any effect runs, and are never destroyed + // (see above), so this always binds the same doc/awareness the editor froze at mount. const doc = docRef.current as Y.Doc const awareness = awarenessRef.current as Awareness const fileProvider = new FileDocProvider(socket, fileId, doc, awareness) @@ -87,7 +110,52 @@ export function useFileDocCollaboration({ } }, [enabled, socket, fileId]) - const user = useMemo(() => ({ name: userName, color: getUserColor(userId) }), [userName, userId]) + const reportOthers = useReportFileDocOthers() + const reportOthersRef = useRef(reportOthers) + reportOthersRef.current = reportOthers + + // "Who's in this file" roster (the useOthers side of the pattern). The server broadcasts a + // roster of SERVER-AUTHENTICATED identities (see FILE_DOC_EVENTS.PRESENCE) — trusted, + // unlike the client-set awareness `user` field a peer could spoof. Publish it (minus self) + // to the room context for the file-detail avatar stack; cleared on unmount so a file switch + // never shows the previous file's occupants. + useEffect(() => { + if (!enabled || !socket) return + const handlePresence = (data: FileDocPresence) => { + if (data.fileId !== fileId) return + // Exclude only our OWN socket (this session), NOT every session that shares our userId — + // so a second tab of the same account still counts as present, matching the canvas avatars + // (avatars.tsx filters by socketId). Then dedupe per user for the display stack, so + // multiple tabs of one person collapse to a single avatar. + const byUser = new Map< + string, + { userId: string; userName: string; avatarUrl: string | null } + >() + for (const peer of data.users) { + if (peer.socketId === socket.id || byUser.has(peer.userId)) continue + byUser.set(peer.userId, { + userId: peer.userId, + userName: peer.userName, + avatarUrl: peer.avatarUrl, + }) + } + reportOthersRef.current([...byUser.values()]) + } + socket.on(FILE_DOC_EVENTS.PRESENCE, handlePresence) + return () => { + socket.off(FILE_DOC_EVENTS.PRESENCE, handlePresence) + reportOthersRef.current([]) + } + }, [enabled, socket, fileId]) + + // The client id rides in the awareness `user` payload so the caret `render` (which only + // receives `user`) can tag each caret node for the activity-driven name label (see + // caret-presence.ts). `doc.clientID` is stable for the doc's life, so reading it from the + // ref needs no memo dep. + const user = useMemo( + () => ({ name: userName, color: getUserColor(userId), clientId: docRef.current?.clientID }), + [userName, userId] + ) return useMemo( () => diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-extensions.ts b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-extensions.ts index 12190e62718..0e75964c427 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-extensions.ts +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-extensions.ts @@ -8,6 +8,11 @@ import { withAlpha } from '@/lib/workspaces/colors' import { BlockMover } from './block-mover' import { CodeBlockWithLanguage } from './code-block' import { CodeBlockHighlight } from './code-highlight' +import { + createCaretActivityExtension, + DEFAULT_CARET_COLOR, + renderCaret, +} from './collaboration/caret-presence' import { LinkEmbed } from './embed/link-embed' import { createMarkdownContentExtensions } from './extensions' import { ResizableImage } from './image' @@ -64,19 +69,22 @@ export function createMarkdownEditorExtensions({ ? [ Collaboration.configure({ document: collaboration.doc }), // CollaborationCaret reads only `provider.awareness` (created synchronously, - // relayed by the socket provider once connected). The default caret + label - // color from `user.color`; only the selection tint needs an explicit override. + // relayed by the socket provider once connected). `render` tags each caret + // with the peer's client id and shows its name label; the selection tint is + // a translucent fill of the peer's identity color. CollaborationCaret.configure({ provider: { awareness: collaboration.awareness }, user: collaboration.user, + render: renderCaret, selectionRender: (user) => { - const hex = typeof user.color === 'string' ? user.color : '#000000' + const hex = typeof user.color === 'string' ? user.color : DEFAULT_CARET_COLOR return { class: 'collaboration-carets__selection', style: `background-color: ${withAlpha(hex, 0.2)};`, } }, }), + createCaretActivityExtension(collaboration.awareness), ] : []), CodeBlockHighlight, diff --git a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css index 2ec9890d8be..ec33493934b 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css +++ b/apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.css @@ -355,12 +355,18 @@ border-radius: 4px; } +/* `overflow: visible` (not `hidden`) so a collaborator's caret name label, which + * pops above the caret, can escape the table box instead of being clipped by it. + * With `table-layout: fixed; width: 100%` columns can't exceed the table, so there + * is nothing to clip here anyway; the only absolutely-positioned descendant is the + * decorative column-resize handle, whose 2px right sliver is clipped by the editor's + * own scroll container. */ .rich-markdown-prose table { width: 100%; border-collapse: collapse; table-layout: fixed; margin: 1rem 0; - overflow: hidden; + overflow: visible; } .rich-markdown-prose th, @@ -456,11 +462,14 @@ } /* - * Collaborative carets (TipTap CollaborationCaret). The caret border and the name - * label's background are colored inline from each user's assigned color; the - * selection is a translucent tint of that color (set via selectionRender). The - * name label is hidden until the caret is hovered — a quiet, Google-Docs-style - * presence that doesn't clutter the text. + * Collaborative carets (TipTap CollaborationCaret). The caret bar and the name + * label's background are colored inline from each collaborator's identity color + * (the same `getUserColor` mechanism the canvas cursors use); the selection is a + * translucent tint of that color (set via selectionRender). The name label shows + * while the peer is active (JS toggles `--active` on each awareness change) or on + * hover, then fades after inactivity — matching Google Docs. `z-index` lifts the + * caret (and its label) above table cell backgrounds so a caret inside a table + * cell is not hidden behind adjacent cells. */ .rich-markdown-prose .collaboration-carets__caret { position: relative; @@ -468,42 +477,98 @@ margin-right: -1px; border-left-width: 1px; border-left-style: solid; + border-left-color: var(--caret-color); border-right-width: 1px; border-right-style: solid; + border-right-color: var(--caret-color); word-break: normal; + z-index: 20; +} + +/* Dormant affordance: a small cap at the top of the caret in the collaborator's color, + * shaped like a collapsed presence name tag (same `rounded-xs` + notch corner as the + * tables/canvas tags) so the whole presence system reads as one language. It signals + * "someone's here — hover for who", and fades out when the full name tag takes over + * (peer active or on hover). */ +.rich-markdown-prose .collaboration-carets__caret::before { + content: ""; + position: absolute; + /* Seat the cap's square bottom-left corner on the pole's top-left, flush like a flag on its + * pole. `left: -1px` backs out the caret's 1px left border (the abs-positioning origin is the + * padding box, inside that border) so the cap's left edge lines up with the pole's left edge; + * `top` overlaps the pole's top a hair so the notch reads as continuous, no gap. */ + top: -2px; + left: -1px; + width: 8px; + height: 5px; + border-radius: 2px 2px 2px 0; + background-color: var(--caret-color); + transition: opacity 0.2s ease; +} + +.rich-markdown-prose .collaboration-carets__caret--active::before, +.rich-markdown-prose .collaboration-carets__caret:hover::before { + opacity: 0; +} + +/* Transparent hover hit-slop. The visible caret bar is only ~2px wide and the dormant cap is + * 8×5px, so "hover for who" (revealing the name label on a dormant caret) would be near + * impossible to trigger. This invisible strip widens the hover target across the caret's full + * height and over the cap — nothing visible changes, only the pointer area. `pointer-events: + * auto` is required so it catches the hover; kept narrow so it barely intrudes on selecting + * text next to a remote caret. */ +.rich-markdown-prose .collaboration-carets__caret::after { + content: ""; + position: absolute; + top: -4px; + bottom: 0; + left: -4px; + right: -4px; + pointer-events: auto; } +/* Matches the canvas cursor name tag (cursors.tsx): identity-color background with + * `--surface-1` text at `text-xs`/`font-medium`, so both presence surfaces read as + * one system. `--surface-1` is the base surface token (readable on every assigned + * identity color in both themes), not a hardcoded value. Hidden by default; the + * show/fade is driven by the `--active` class (see caret-presence.ts) and hover. */ .rich-markdown-prose .collaboration-carets__label { position: absolute; top: -1.4em; left: -1px; + max-width: 10rem; + overflow: hidden; padding: 0.1rem 0.35rem; - border-radius: 3px 3px 3px 0; + border-radius: 2px 2px 2px 0; + /* 11px = the `text-xs` the canvas/tables presence tags use, for pixel parity. */ font-size: 11px; - font-weight: 600; + font-weight: 500; line-height: 1.2; white-space: nowrap; - /* Fixed dark text: the label background is always a light user color (assigned - * inline, theme-independent), so a theme token would go unreadable in one mode. */ - color: #1a1a1a; + text-overflow: ellipsis; + background-color: var(--caret-color); + color: var(--surface-1); user-select: none; pointer-events: none; opacity: 0; - transition: opacity 0.12s ease; -} - -/* Widen the caret's hover target beyond its 1px width so the name label is easy to - * reveal; the label itself stays pointer-events:none and never intercepts clicks. */ -.rich-markdown-prose .collaboration-carets__caret::before { - content: ""; - position: absolute; - inset: -0.1em -2px; + transition: opacity 0.2s ease; } +.rich-markdown-prose .collaboration-carets__caret--active .collaboration-carets__label, .rich-markdown-prose .collaboration-carets__caret:hover .collaboration-carets__label { opacity: 1; } +/* Near the editor's right edge the label is flipped to the caret's left so it never + * runs off (JS toggles `--flip` after measuring); mirror the tag's notch corner. */ +.rich-markdown-prose .collaboration-carets__caret--flip .collaboration-carets__label { + left: auto; + right: -1px; + border-radius: 2px 2px 0 2px; +} + +/* Remote text selection: a rounded translucent tint of the collaborator's identity + * color (the alpha fill is set inline by selectionRender). */ .rich-markdown-prose .collaboration-carets__selection { border-radius: 2px; pointer-events: none; diff --git a/apps/sim/app/workspace/[workspaceId]/files/files.tsx b/apps/sim/app/workspace/[workspaceId]/files/files.tsx index 29531265471..8094080b93d 100644 --- a/apps/sim/app/workspace/[workspaceId]/files/files.tsx +++ b/apps/sim/app/workspace/[workspaceId]/files/files.tsx @@ -61,7 +61,6 @@ import { Resource, timeCell, } from '@/app/workspace/[workspaceId]/components' -import { PresenceAvatars } from '@/app/workspace/[workspaceId]/components/presence/presence-avatars' import { FilesActionBar } from '@/app/workspace/[workspaceId]/files/components/action-bar' import { DeleteConfirmModal } from '@/app/workspace/[workspaceId]/files/components/delete-confirm-modal' import { FileRowContextMenu } from '@/app/workspace/[workspaceId]/files/components/file-row-context-menu' @@ -73,6 +72,8 @@ import { isPreviewable, isTextEditable, } from '@/app/workspace/[workspaceId]/files/components/file-viewer' +import { FileDocAvatars } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-avatars' +import { FileDocRoomProvider } from '@/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/collaboration/file-doc-room-context' import { FilesListContextMenu } from '@/app/workspace/[workspaceId]/files/components/files-list-context-menu' import { ShareModal } from '@/app/workspace/[workspaceId]/files/components/share-modal' import { useWorkspaceFilesRoom } from '@/app/workspace/[workspaceId]/files/hooks/use-workspace-files-room' @@ -205,10 +206,10 @@ export function Files() { const canEdit = userPermissions.canEdit === true const { config: permissionConfig } = usePermissionConfig() - const { otherUsers: filesPresenceUsers } = useWorkspaceFilesRoom( - workspaceId, - currentFolderId ?? null - ) + // Joined for the live file tree: a `workspace-files-changed` broadcast invalidates the + // browser. "Who's in this file" comes from the file-doc room (see FileDocRoomProvider), + // not from who's browsing the Files section. + useWorkspaceFilesRoom(workspaceId) useEffect(() => { if (permissionConfig.hideFilesTab) { @@ -1907,36 +1908,42 @@ export function Files() { if (selectedFile) { return ( <> - - - + {/* The room provider scopes "who's in this file" presence to the open document: the + editor (inside FileViewer) publishes the server-authenticated roster and the + header's FileDocAvatars reads it — both must be descendants. */} + + + } + /> + - - + + + } /> ([]) - - const tabSessionIdRef = useRef('') - if (!tabSessionIdRef.current) tabSessionIdRef.current = generateShortId() - const folderIdRef = useRef(folderId) - - useEffect(() => { - folderIdRef.current = folderId - }, [folderId]) - useEffect(() => { if (!socket || !workspaceId) return let retries = 0 let retryTimer: ReturnType | null = null - const join = () => { - socket.emit('join-workspace-files', { - workspaceId, - folderId: folderIdRef.current, - tabSessionId: tabSessionIdRef.current, - }) - } + const join = () => socket.emit('join-workspace-files', { workspaceId }) - const handleJoinSuccess = (data: { - workspaceId: string - presenceUsers: PresenceUpdatePayload[] - }) => { + const handleJoinSuccess = (data: { workspaceId: string }) => { if (data.workspaceId !== workspaceId) return retries = 0 - // Cancel any retry scheduled by a prior retryable error so it can't fire an - // extra join after we're already in. + // Cancel any retry scheduled by a prior retryable error so it can't fire an extra + // join after we're already in. if (retryTimer) { clearTimeout(retryTimer) retryTimer = null } - setPresenceUsers(data.presenceUsers ?? []) } const handleJoinError = (data: JoinErrorPayload) => { if (data.workspaceId !== workspaceId) return @@ -93,7 +58,6 @@ export function useWorkspaceFilesRoom( retryTimer = setTimeout(join, JOIN_RETRY_BASE_MS * retries) } } - const handlePresence = (users: PresenceUpdatePayload[]) => setPresenceUsers(users ?? []) const handleChanged = (data: { workspaceId: string }) => { if (data.workspaceId === workspaceId) invalidateWorkspaceFileBrowsers(queryClient, workspaceId) @@ -104,7 +68,6 @@ export function useWorkspaceFilesRoom( socket.on('connect', join) socket.on('join-workspace-files-success', handleJoinSuccess) socket.on('join-workspace-files-error', handleJoinError) - socket.on('workspace-files:presence-update', handlePresence) socket.on('workspace-files-changed', handleChanged) return () => { @@ -112,22 +75,12 @@ export function useWorkspaceFilesRoom( socket.off('connect', join) socket.off('join-workspace-files-success', handleJoinSuccess) socket.off('join-workspace-files-error', handleJoinError) - socket.off('workspace-files:presence-update', handlePresence) socket.off('workspace-files-changed', handleChanged) - setPresenceUsers([]) - // Leave the room, scoped to THIS workspace: the server no-ops if the socket - // has already switched to another workspace's files room (so a workspace - // A→B switch, where B's join runs first and auto-leaves A, can't have A's - // leave evict the fresh B membership). + // Leave the room, scoped to THIS workspace: the server no-ops if the socket has + // already switched to another workspace's files room (so a workspace A→B switch, + // where B's join runs first and auto-leaves A, can't have A's leave evict B). socket.emit('leave-workspace-files', { workspaceId }) } }, [socket, workspaceId, queryClient]) - - const otherUsers = useMemo( - () => presenceUsers.filter((user) => user.socketId !== currentSocketId), - [presenceUsers, currentSocketId] - ) - - return { otherUsers } } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx index 74ed697f76a..5461082f3de 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/remote-selection-overlay.tsx @@ -1,7 +1,12 @@ 'use client' import { useCallback, useEffect, useLayoutEffect, useRef, useState } from 'react' +import { createPortal } from 'react-dom' import { getUserColor, withAlpha } from '@/lib/workspaces/colors' +import { + isCellInSelection, + type NormalizedSelection, +} from '@/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils' import type { RemoteTableSelection } from '@/app/workspace/[workspaceId]/tables/[tableId]/hooks/use-table-room' /** A measured remote selection, positioned in the grid content wrapper's space. */ @@ -14,12 +19,26 @@ interface SelectionBox { left: number width: number height: number + /** Viewport-space top/left of the selection, for the body-portaled name label. */ + viewportTop: number + viewportLeft: number + /** Resolved anchor/focus cell indices (undefined when off-window). Coverage by the local + * selection is derived from these in render (see {@link isSelectionCovered}) — no DOM + * re-measure when only the local caret moves. */ + anchorRow: number | undefined + anchorCol: number | undefined + focusRow: number | undefined + focusCol: number | undefined } interface RemoteSelectionOverlayProps { remoteSelections: RemoteTableSelection[] /** Column id → its rendered column index (matches the cells' `data-col`). */ columnIndexById: Map + /** Row id → its index in the current row list, to test local-selection coverage. */ + rowIndexById: Map + /** The local user's own normalized selection, so a co-selected remote cell defers to it. */ + localSelection: NormalizedSelection | null /** The grid's scroll container (`data-table-scroll`), queried for cell rects. */ scrollElement: HTMLElement | null } @@ -39,6 +58,24 @@ function cellRect( return cell?.getBoundingClientRect() } +/** + * Whether a remote selection defers to the local one: true only when the local selection + * fully contains it (both corners inside), so a partial overlap still shows. + */ +function isSelectionCovered( + anchorRow: number | undefined, + anchorCol: number | undefined, + focusRow: number | undefined, + focusCol: number | undefined, + bounds: NormalizedSelection | null +): boolean { + return ( + bounds !== null && + isCellInSelection(anchorRow, anchorCol, bounds) && + isCellInSelection(focusRow, focusCol, bounds) + ) +} + /** * Renders remote collaborators' cell selections over the table grid — a colored * border per user (Google-Sheets style), a darker fill while they are editing, and @@ -54,6 +91,8 @@ function cellRect( export function RemoteSelectionOverlay({ remoteSelections, columnIndexById, + rowIndexById, + localSelection, scrollElement, }: RemoteSelectionOverlayProps) { const rootRef = useRef(null) @@ -68,6 +107,11 @@ export function RemoteSelectionOverlay({ remoteSelectionsRef.current = remoteSelections const columnIndexByIdRef = useRef(columnIndexById) columnIndexByIdRef.current = columnIndexById + const rowIndexByIdRef = useRef(rowIndexById) + rowIndexByIdRef.current = rowIndexById + // Read only by the pointer hit-test (never in render) to skip a locally-covered box. + const localSelectionRef = useRef(localSelection) + localSelectionRef.current = localSelection // Cached content-wrapper origin, refreshed on each measure (scroll/resize/data change), // so the pointer hit-test never forces a layout read per mouse move. const originRef = useRef({ top: 0, left: 0 }) @@ -81,14 +125,20 @@ export function RemoteSelectionOverlay({ const next: SelectionBox[] = [] for (const selection of remoteSelectionsRef.current) { const { anchor, focus, editing } = selection.cell + const anchorCol = columnIndexByIdRef.current.get(anchor.columnId) + const focusCol = columnIndexByIdRef.current.get(focus.columnId) + const anchorRow = rowIndexByIdRef.current.get(anchor.rowId) + const focusRow = rowIndexByIdRef.current.get(focus.rowId) const rects = [ - cellRect(scrollEl, anchor.rowId, columnIndexByIdRef.current.get(anchor.columnId)), - cellRect(scrollEl, focus.rowId, columnIndexByIdRef.current.get(focus.columnId)), + cellRect(scrollEl, anchor.rowId, anchorCol), + cellRect(scrollEl, focus.rowId, focusCol), ].filter((rect): rect is DOMRect => rect !== undefined) if (rects.length === 0) continue - const top = Math.min(...rects.map((r) => r.top)) - origin.top - const left = Math.min(...rects.map((r) => r.left)) - origin.left + const viewportTop = Math.min(...rects.map((r) => r.top)) + const viewportLeft = Math.min(...rects.map((r) => r.left)) + const top = viewportTop - origin.top + const left = viewportLeft - origin.left const bottom = Math.max(...rects.map((r) => r.bottom)) - origin.top const right = Math.max(...rects.map((r) => r.right)) - origin.left next.push({ @@ -100,6 +150,12 @@ export function RemoteSelectionOverlay({ left, width: right - left, height: bottom - top, + viewportTop, + viewportLeft, + anchorRow, + anchorCol, + focusRow, + focusCol, }) } setBoxes(next) @@ -124,7 +180,20 @@ export function RemoteSelectionOverlay({ const x = event.clientX - left const y = event.clientY - top const hit = boxesRef.current.find( - (b) => x >= b.left && x <= b.left + b.width && y >= b.top && y <= b.top + b.height + (b) => + x >= b.left && + x <= b.left + b.width && + y >= b.top && + y <= b.top + b.height && + // Skip a box the local selection covers — it isn't drawn, so hovering it must not + // pop a name tag over a cell with no visible remote selection. + !isSelectionCovered( + b.anchorRow, + b.anchorCol, + b.focusRow, + b.focusCol, + localSelectionRef.current + ) ) setHoveredSocketId((prev) => prev === (hit?.socketId ?? null) ? prev : (hit?.socketId ?? null) @@ -160,37 +229,84 @@ export function RemoteSelectionOverlay({ } }, [scrollElement, measure]) - // Re-measure when the selections or column layout change (listeners stay subscribed). - // Layout effect so positions update before paint — no one-frame lag as a peer moves. + // Re-measure when the remote selections or column layout change (listeners stay + // subscribed). Layout effect so positions update before paint — no one-frame lag as a + // peer moves. NOT keyed on `localSelection`: moving the local caret changes only which + // boxes are `covered`, which the cheap in-memory pass below handles without a reflow. useLayoutEffect(() => { measure() }, [remoteSelections, columnIndexById, measure]) + // Re-derived in render so it reacts to `localSelection`: when the local selection grows to + // cover the hovered box (its outline is no longer drawn) without another pointer move, the + // floating name tag must drop rather than linger over cells with no visible remote selection. + const hoveredBox = hoveredSocketId + ? boxes.find( + (box) => + box.socketId === hoveredSocketId && + !isSelectionCovered( + box.anchorRow, + box.anchorCol, + box.focusRow, + box.focusCol, + localSelection + ) + ) + : undefined + return ( -
- {boxes.map((box) => ( -
- {hoveredSocketId === box.socketId && ( - - {box.userName} - - )} -
- ))} -
+ <> +
+ {boxes.map((box) => + // A cell the local user also has selected shows only the local selection — the + // remote box isn't drawn (its `boxes` entry still drives the hover name). The + // border is an inset box-shadow (no layout width, so it never stacks with an + // adjacent cell's border) plus a subtle fill, darker while the peer is editing. + isSelectionCovered( + box.anchorRow, + box.anchorCol, + box.focusRow, + box.focusCol, + localSelection + ) ? null : ( +
+ ) + )} +
+ {/* The name label portals to the body so it floats on top of the grid (and its + sticky header) instead of being clipped by the overlay's overflow-hidden; it's + placed in viewport space, its bottom-left tabbed onto the selection's top-left. */} + {hoveredBox && + createPortal( + // Same chrome as the workflow-canvas presence label (see cursors.tsx): the + // identity color is the only per-user value; text color, font, radius, and + // padding all reuse the canvas tokens so tables + canvas presence look identical. + // `rounded-bl-none` tabs the label's bottom-left corner onto the selection's + // top-left, the one deviation from the free-floating canvas cursor tag. +
+ {hoveredBox.userName} +
, + document.body + )} + ) } diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx index 8ae1abd1df7..821018b6692 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/table-grid.tsx @@ -62,6 +62,7 @@ import { computeNormalizedSelection, type ExecStatusMix, expandToDisplayColumns, + isCellInSelection, moveCell, ROW_SELECTION_ALL, ROW_SELECTION_NONE, @@ -663,6 +664,15 @@ export function TableGrid({ return map }, [displayColumns]) + /** Row id → its index in the current row list, for testing local-selection coverage. + * Only built when collaborators are present (the overlay is gated on that too), so + * solo editing never pays the O(n) map build on a refetch. */ + const rowIndexById = useMemo(() => { + const map = new Map() + if (remoteSelections.length > 0) rows.forEach((row, index) => map.set(row.id, index)) + return map + }, [rows, remoteSelections.length]) + const workflowGroupById = useMemo( () => new Map(tableWorkflowGroups.map((g) => [g.id, g])), [tableWorkflowGroups] @@ -1211,12 +1221,7 @@ export function TableGrid({ selectionAnchorRef.current, selectionFocusRef.current ) - const isWithinSelection = - sel !== null && - rowIndex >= sel.startRow && - rowIndex <= sel.endRow && - colIndex >= sel.startCol && - colIndex <= sel.endCol + const isWithinSelection = sel !== null && isCellInSelection(rowIndex, colIndex, sel) if (!isWithinSelection) { setSelectionAnchor({ rowIndex, colIndex }) @@ -3943,6 +3948,8 @@ export function TableGrid({ )} diff --git a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts index da0e5674751..788d11f6579 100644 --- a/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts +++ b/apps/sim/app/workspace/[workspaceId]/tables/[tableId]/components/table-grid/utils.ts @@ -83,6 +83,25 @@ export interface NormalizedSelection { anchorCol: number } +/** + * Whether a (row, col) index pair falls inside a normalized selection rectangle. + * Row/col may be `undefined` (e.g. an id that didn't resolve to an index) → `false`. + */ +export function isCellInSelection( + row: number | undefined, + col: number | undefined, + sel: NormalizedSelection +): boolean { + return ( + row !== undefined && + col !== undefined && + row >= sel.startRow && + row <= sel.endRow && + col >= sel.startCol && + col <= sel.endCol + ) +} + /** A run of consecutive `displayColumns` rendered together in the meta header row. */ export type HeaderGroup = | { kind: 'plain'; size: 1; startColIndex: number } diff --git a/packages/realtime-protocol/src/file-doc.ts b/packages/realtime-protocol/src/file-doc.ts index 738af5bc123..9d2fbe3a5db 100644 --- a/packages/realtime-protocol/src/file-doc.ts +++ b/packages/realtime-protocol/src/file-doc.ts @@ -32,6 +32,13 @@ export const FILE_DOC_EVENTS = { LEAVE: 'leave-file-doc', /** Both directions: a framed Yjs message (binary), tagged by {@link FILE_DOC_MESSAGE_TYPE}. */ MESSAGE: 'file-doc-message', + /** + * Server → client: the roster of collaborators currently in the document + * ({@link FileDocPresence}), for the avatar stack. Identity is server-authenticated (from + * each socket's session), so — unlike the client-set awareness `user` field — a peer + * cannot spoof or suppress another user's entry. Re-sent on every join and leave. + */ + PRESENCE: 'file-doc-presence', } as const /** @@ -121,6 +128,25 @@ export interface LeaveFileDocPayload { fileId: string } +/** One collaborator session in a {@link FileDocPresence} roster — server-authenticated identity. + * Keyed per socket (session), not per user: the client excludes its OWN `socketId` and then + * dedupes the rest per user for the avatar stack, so a second tab of the same account still + * registers as present (mirroring the canvas presence model). */ +export interface FileDocPresenceUser { + /** The collaborator's socket id, so a client can exclude its own session (not every session + * that happens to share its userId). */ + socketId: string + userId: string + userName: string + avatarUrl: string | null +} + +/** Server → client roster of who is in the document ({@link FILE_DOC_EVENTS.PRESENCE}). */ +export interface FileDocPresence { + fileId: string + users: FileDocPresenceUser[] +} + /** * Coerce a Socket.IO binary payload to a `Uint8Array`, or `null` if it is * neither. Shared by the server relay and the client provider so the two agree