From 9375f9f4eddfa15135dca5d43159d0b83ce7921c Mon Sep 17 00:00:00 2001 From: Alexey Taktarov Date: Fri, 9 Oct 2026 15:52:40 +0200 Subject: [PATCH 1/3] Replace per-workspace host state with one global publishUsers call An outer host now hands moi its identity and users through a single call, window.moi.collab.publishUsers({ me, users }). The same call covers startup and every later change, and each call replaces the whole snapshot. - The user directory is global to the moi instance instead of one list per workspace, and it no longer has a loading state. A published list is authoritative, including when it is empty. - `me` is an id that must be listed in `users`. Leaving it out, or passing null, means nobody is signed in. The directory is then emptied even if the host still lists users. - moi owns the state and the host only pushes into it. setHostState, getHostState, subscribeHostState, the getter read before load, and the moi:collab-ready event are removed. - installHostApi() runs while the entry module evaluates rather than inside the async init, so a host module script placed after the bundle finds the API at once and publishes before React mounts. A classic inline script in that position runs before the bundle and finds nothing. - Current-user observers are notified only when the viewer or its source actually changed. Each notice makes every open workspace resend the viewer profile to its room, so a directory update for other users now sends nothing over the socket. - The engine contract exposes getDirectory and subscribeDirectory, and the default engine reads the same global directory, so users also resolve outside a workspace. The applet API is unchanged. The applet guide drops the loading directory and notes that the directory is the same in every workspace. Co-Authored-By: Claude Opus 5.5 --- client/features/collab/client.test.ts | 89 ++-- client/features/collab/client.ts | 21 +- client/features/collab/hooks.test.tsx | 46 +- client/features/collab/hooks.tsx | 17 +- client/features/collab/host-state.test.ts | 467 +++++++----------- client/features/collab/host-state.ts | 185 +++---- client/features/collab/testing/fake-engine.ts | 33 +- .../collab/workspace-users-menu.test.tsx | 24 +- .../features/collab/workspace-users-menu.tsx | 8 +- client/index.tsx | 6 +- .../skills/moi-workspace/references/COLLAB.md | 9 +- 11 files changed, 375 insertions(+), 530 deletions(-) diff --git a/client/features/collab/client.test.ts b/client/features/collab/client.test.ts index 3fd54434..87b9ce12 100644 --- a/client/features/collab/client.test.ts +++ b/client/features/collab/client.test.ts @@ -6,7 +6,8 @@ import type { CollabClientMessage } from '@/lib/collab/types' import { CollabService } from '@/server/collab/service' import { CollabClient, NO_ENGINE } from './client' -import { getCurrentUser, setHostState } from './host-state' +import { getCurrentUser, publishUsers } from './host-state' +import type { UserProfileInput } from './host-state' const originalLocation = Object.getOwnPropertyDescriptor(globalThis, 'location') const originalWebSocket = Object.getOwnPropertyDescriptor(globalThis, 'WebSocket') @@ -14,6 +15,14 @@ let service: CollabService let sockets: TestSocket[] let stop: (() => void) | undefined +// Publish as an outer host would: one viewer, listed in the directory. +function signIn(user: UserProfileInput, others: readonly UserProfileInput[] = []) { + publishUsers({ me: user.id, users: [user, ...others] }) +} +function signOut() { + publishUsers({ users: [] }) +} + class TestSocket { static OPEN = 1 readyState = 0 @@ -56,7 +65,7 @@ class TestSocket { } beforeEach(() => { - setHostState({ currentUser: undefined, workspaces: {} }) + signOut() sockets = [] service = new CollabService((connectionId, message) => { sockets @@ -74,7 +83,7 @@ afterEach(() => { stop?.() stop = undefined service.close() - setHostState({ currentUser: undefined, workspaces: {} }) + signOut() if (originalLocation) Object.defineProperty(globalThis, 'location', originalLocation) else Reflect.deleteProperty(globalThis, 'location') if (originalWebSocket) Object.defineProperty(globalThis, 'WebSocket', originalWebSocket) @@ -100,22 +109,22 @@ test('an explicit user enables workspace presence without an applet and clearing const engine = new CollabClient('workspace') stop = engine.start() const profile = { id: 'alice', name: 'Alice', color: 'emerald' } as const - setHostState({ currentUser: profile, workspaces: {} }) + signIn(profile) expect(sockets).toHaveLength(1) const socket = sockets[0]! socket.open() expect(socket.sent[0]).toMatchObject({ type: 'join', profile }) expect(engine.getSnapshot().connections[0]?.userId).toEqual(profile.id) - setHostState({ currentUser: { ...profile, name: 'Alicia' }, workspaces: {} }) + signIn({ ...profile, name: 'Alicia' }) expect(socket.sent.at(-1)).toMatchObject({ type: 'profile', profile: { name: 'Alicia' } }) expect(sockets).toHaveLength(1) - setHostState({ currentUser: undefined, workspaces: {} }) + signOut() expect(socket.readyState).toBe(3) expect(engine.getSnapshot().status).toBe('disconnected') }) test('two tabs send the same resolved color for a host user without one', () => { - setHostState({ currentUser: { id: 'alice' }, workspaces: {} }) + signIn({ id: 'alice' }) const first = new CollabClient('workspace') const second = new CollabClient('workspace') stop = first.start() @@ -135,7 +144,7 @@ test('two tabs send the same resolved color for a host user without one', () => }) test('reconnecting restores current presence and removed registrations stay gone', async () => { - setHostState({ currentUser: { id: 'alice', name: 'Alice', color: 'emerald' }, workspaces: {} }) + signIn({ id: 'alice', name: 'Alice', color: 'emerald' }) const engine = new CollabClient('workspace') stop = engine.start() sockets[0]!.open() @@ -176,7 +185,7 @@ test('a rejected join retries after capacity frees and cleanup cancels pending w profile: { id: `occupied-${index}`, color: 'blue' } }) } - setHostState({ currentUser: { id: 'alice', color: 'emerald' }, workspaces: {} }) + signIn({ id: 'alice', color: 'emerald' }) const heartbeat = spyOn(globalThis, 'setInterval') const clearHeartbeat = spyOn(globalThis, 'clearInterval') const timer = spyOn(globalThis, 'setTimeout') @@ -231,7 +240,7 @@ test('a rejected join retries after capacity frees and cleanup cancels pending w }) test('errors after joining keep the connection available for further presence', () => { - setHostState({ currentUser: { id: 'alice', color: 'emerald' }, workspaces: {} }) + signIn({ id: 'alice', color: 'emerald' }) const client = new CollabClient('workspace') stop = client.start() const socket = sockets[0]! @@ -250,24 +259,38 @@ test('errors after joining keep the connection available for further presence', test('profile edits publish immediately without changing the user id or reconnecting', () => { const alice = { id: 'alice', name: 'Alice', color: 'emerald' } as const - setHostState({ currentUser: alice, workspaces: {} }) + signIn(alice) const engine = new CollabClient('workspace') stop = engine.start() sockets[0]!.open() const edited = { ...alice, name: 'Ada Lovelace', color: 'cyan' } as const - setHostState({ currentUser: edited, workspaces: {} }) + signIn(edited) expect(sockets).toHaveLength(1) expect(sockets[0]!.readyState).toBe(TestSocket.OPEN) expect(sockets[0]!.sent.at(-1)).toEqual({ type: 'profile', profile: edited }) expect(engine.getSnapshot().users).toEqual([edited]) }) +test('a directory change that leaves the viewer alone sends nothing to the room', () => { + const alice = { id: 'alice', name: 'Alice', color: 'emerald' } as const + const bob = { id: 'bob', name: 'Bob', color: 'blue' } as const + signIn(alice, [bob]) + const engine = new CollabClient('workspace') + stop = engine.start() + sockets[0]!.open() + const sent = sockets[0]!.sent.length + signIn(alice, [{ ...bob, name: 'Robert' }]) + expect(engine.getDirectory()).toEqual([alice, { ...bob, name: 'Robert' }]) + expect(sockets[0]!.sent).toHaveLength(sent) + expect(sockets).toHaveLength(1) +}) + test('changing user reconnects and never sends the host directory over the socket', async () => { - setHostState({ currentUser: { id: 'alice', name: 'Alice', color: 'emerald' }, workspaces: {} }) + signIn({ id: 'alice', name: 'Alice', color: 'emerald' }) const engine = new CollabClient('workspace') stop = engine.start() sockets[0]!.open() - setHostState({ currentUser: { id: 'bob', name: 'Bob', color: 'blue' }, workspaces: {} }) + signIn({ id: 'bob', name: 'Bob', color: 'blue' }) expect(sockets[0]!.readyState).toBe(3) await Bun.sleep(550) sockets[1]!.open() @@ -275,25 +298,20 @@ test('changing user reconnects and never sends the host directory over the socke expect(sockets[1]!.sent.every(message => !('users' in message))).toBe(true) }) -test('disabled engine exposes host profiles and readiness without starting a transport', () => { +test('disabled engine exposes host profiles without starting a transport', () => { const alice = { id: 'alice', name: 'Alice', color: 'emerald' } as const - setHostState({ - currentUser: alice, - workspaces: { workspace: { status: 'loading' } } - }) + const bob = { id: 'bob', name: 'Bob', color: 'blue' } as const + signIn(alice) const engine = new CollabClient('workspace', false) stop = engine.start() expect(engine.enabled).toBe(false) expect(engine.workspaceId).toBe('workspace') expect(engine.getCurrentUser()).toEqual(alice) - expect(engine.getWorkspaceDirectory()).toEqual({ status: 'loading' }) + expect(engine.getDirectory()).toEqual([alice]) let updates = 0 - const unsubscribe = engine.subscribeWorkspaceUsers(() => updates++) - setHostState({ - currentUser: alice, - workspaces: { workspace: { status: 'ready', users: [alice] } } - }) - expect(engine.getWorkspaceDirectory()).toEqual({ status: 'ready', users: [alice] }) + const unsubscribe = engine.subscribeDirectory(() => updates++) + signIn(alice, [bob]) + expect(engine.getDirectory()).toEqual([alice, bob]) expect(updates).toBe(1) engine.setLocation({ page: 'overview', status: 'active' }) engine.setPresence({ @@ -312,15 +330,15 @@ test('disabled engine exposes host profiles and readiness without starting a tra test('engine cleanup survives remounting and does not leave user listeners or retry sockets', async () => { const alice = { id: 'alice', name: 'Alice', color: 'emerald' } as const - setHostState({ currentUser: alice, workspaces: {} }) + signIn(alice) const engine = new CollabClient('workspace') stop = engine.start() engine.start()() sockets[0]!.open() - setHostState({ currentUser: { ...alice, name: 'Alicia' }, workspaces: {} }) + signIn({ ...alice, name: 'Alicia' }) expect(sockets[0]!.sent.filter(message => message.type === 'profile')).toHaveLength(1) stop() - setHostState({ currentUser: { ...alice, name: 'Alice again' }, workspaces: {} }) + signIn({ ...alice, name: 'Alice again' }) await Bun.sleep(550) expect(sockets).toHaveLength(1) stop = engine.start() @@ -329,11 +347,18 @@ test('engine cleanup survives remounting and does not leave user listeners or re expect(engine.getSnapshot().status).toBe('connected') }) -test('the default engine keeps the current user readable while workspace operations stay inert', () => { +test('the default engine keeps the viewer and directory readable while workspace operations stay inert', () => { const alice = { id: 'alice', name: 'Alice', color: 'emerald' } as const - setHostState({ currentUser: alice, workspaces: {} }) + const bob = { id: 'bob', name: 'Bob', color: 'blue' } as const + signIn(alice, [bob]) expect(NO_ENGINE.getCurrentUser()).toEqual(alice) - expect(NO_ENGINE.getWorkspaceDirectory()).toBeUndefined() + // The directory belongs to the app, not to a workspace, so it needs no engine. + expect(NO_ENGINE.getDirectory()).toEqual([alice, bob]) + let updates = 0 + const unsubscribe = NO_ENGINE.subscribeDirectory(() => updates++) + signIn(alice) + unsubscribe() + expect([NO_ENGINE.getDirectory(), updates]).toEqual([[alice], 1]) expect(NO_ENGINE.getPresenceSnapshot('one', 'field')).toBe( NO_ENGINE.getPresenceSnapshot('two', 'cursor') ) diff --git a/client/features/collab/client.ts b/client/features/collab/client.ts index 467a8f52..74e28b30 100644 --- a/client/features/collab/client.ts +++ b/client/features/collab/client.ts @@ -10,11 +10,10 @@ import type { import { getCurrentUser, - getWorkspaceDirectory, + getDirectory, subscribeCurrentUserStore, - subscribeWorkspaceUsersStore + subscribeDirectoryStore } from './host-state' -import type { WorkspaceDirectory } from './host-state' import { CollabStore, DISCONNECTED_STATE } from './store' import type { ClientState, PresenceEntry } from './store' @@ -32,8 +31,10 @@ export type CollabEngineApi = { subscribe: (listener: () => void) => Unsubscribe getCurrentUser: () => UserProfile | undefined subscribeCurrentUser: (listener: () => void) => Unsubscribe - getWorkspaceDirectory: () => WorkspaceDirectory | undefined - subscribeWorkspaceUsers: (listener: () => void) => Unsubscribe + // Everyone an outer host published. Undefined without a host, in which + // case live users stand in. + getDirectory: () => readonly UserProfile[] | undefined + subscribeDirectory: (listener: () => void) => Unsubscribe getLocation: () => ConnectionLocation | null setLocation: (location: ConnectionLocation | null) => void setPresence: (registration: PresenceRegistration) => void @@ -53,8 +54,8 @@ export const NO_ENGINE: CollabEngineApi = { subscribe: () => noop, getCurrentUser, subscribeCurrentUser: subscribeCurrentUserStore, - getWorkspaceDirectory: () => undefined, - subscribeWorkspaceUsers: () => noop, + getDirectory, + subscribeDirectory: subscribeDirectoryStore, getLocation: () => null, setLocation: noop, setPresence: noop, @@ -91,10 +92,8 @@ export class CollabClient implements CollabEngineApi { this.enabled ? this.store.subscribe(listener) : noop getCurrentUser = getCurrentUser subscribeCurrentUser = subscribeCurrentUserStore - getWorkspaceDirectory = (): WorkspaceDirectory | undefined => - getWorkspaceDirectory(this.workspaceId) - subscribeWorkspaceUsers = (listener: () => void): Unsubscribe => - subscribeWorkspaceUsersStore(this.workspaceId, listener) + getDirectory = getDirectory + subscribeDirectory = subscribeDirectoryStore getLocation = (): ConnectionLocation | null => (this.enabled ? this.store.getLocation() : null) setLocation = (location: ConnectionLocation | null): void => { this.store.setLocation(location) diff --git a/client/features/collab/hooks.test.tsx b/client/features/collab/hooks.test.tsx index 0d60b998..91a39b36 100644 --- a/client/features/collab/hooks.test.tsx +++ b/client/features/collab/hooks.test.tsx @@ -2,7 +2,7 @@ import { expect, test } from 'bun:test' import { renderToStaticMarkup } from 'react-dom/server' import type { CollabEngineApi } from './client' import { createFakeEngine } from './testing/fake-engine' -import { setHostState } from './host-state' +import { publishUsers } from './host-state' import { useMe, usePeers, @@ -10,8 +10,7 @@ import { usePublishPresence, useUser, useUsers, - useWorkspaceUsers, - useWorkspaceDirectory + useWorkspaceUsers } from './hooks' import { AppletPresenceProvider, CollabContext, pageFromPath } from './provider' import type { WorkspaceUser } from './hooks' @@ -95,11 +94,33 @@ function Disabled() { return } test('hooks are safe without a backend or applet and resolve missing users to undefined', () => { - setHostState({ currentUser: undefined, workspaces: {} }) + publishUsers({ users: [] }) const html = renderToStaticMarkup() expect(html).toContain('[]') }) +test('a published directory resolves users without a workspace engine', () => { + publishUsers({ me: 'alice', users: [alice, bob] }) + try { + function Global() { + return encodeURIComponent( + JSON.stringify({ me: useMe(), user: useUser('bob'), users: useWorkspaceUsers() }) + ) + } + // No room is joined here, so everyone resolves and reads as offline. + expect(JSON.parse(decodeURIComponent(renderToStaticMarkup()))).toEqual({ + me: { ...alice, status: 'offline' }, + user: { ...bob, status: 'offline' }, + users: [ + { ...alice, status: 'offline' }, + { ...bob, status: 'offline' } + ] + }) + } finally { + publishUsers({ users: [] }) + } +}) + test('user hooks preserve missing names for self, peers, and offline members', () => { const self = { id: 'self', color: 'emerald', email: 'self@example.test' } as const const peer = { id: 'bob', color: 'blue', email: 'bob@example.test' } as const @@ -215,7 +236,7 @@ test('workspace users include self and offline members with workspace-wide statu expect(render().all).toEqual([]) }) -test('absent, loading, and empty directories preserve distinct user resolution', () => { +test('absent and empty directories preserve distinct user resolution', () => { const engine = createFakeEngine({ self: alice, users: [alice, bob], @@ -225,12 +246,7 @@ test('absent, loading, and empty directories preserve distinct user resolution', function Directory() { const me = useMe() return encodeURIComponent( - JSON.stringify({ - status: useWorkspaceDirectory()?.status, - users: useWorkspaceUsers(), - me, - hasMe: me !== undefined - }) + JSON.stringify({ users: useWorkspaceUsers(), me, hasMe: me !== undefined }) ) } const render = () => @@ -251,13 +267,13 @@ test('absent, loading, and empty directories preserve distinct user resolution', me: { ...alice, status: 'active' }, hasMe: true } + // Without a host directory, the viewer and live users stand in. expect(render()).toEqual(fallback) - engine.setDirectoryStatus('loading') - expect(render()).toEqual({ status: 'loading', users: [], hasMe: false }) + // An empty host directory is authoritative, even over the viewer. engine.setUsers([]) - expect(render()).toEqual({ status: 'ready', users: [], hasMe: false }) + expect(render()).toEqual({ users: [], hasMe: false }) engine.setUsers([alice, bob]) - expect(render()).toEqual({ status: 'ready', ...fallback }) + expect(render()).toEqual(fallback) engine.setUsers(undefined) expect(render()).toEqual(fallback) }) diff --git a/client/features/collab/hooks.tsx b/client/features/collab/hooks.tsx index 4b5f5542..8eb2c20a 100644 --- a/client/features/collab/hooks.tsx +++ b/client/features/collab/hooks.tsx @@ -20,14 +20,6 @@ export function useConnectionState() { const engine = useCollabEngine() return useSyncExternalStore(engine.subscribe, engine.getUsersSnapshot, engine.getUsersSnapshot) } -export function useWorkspaceDirectory() { - const engine = useCollabEngine() - return useSyncExternalStore( - engine.subscribeWorkspaceUsers, - engine.getWorkspaceDirectory, - engine.getWorkspaceDirectory - ) -} function useUsersSource() { const engine = useCollabEngine() const state = useConnectionState() @@ -36,10 +28,13 @@ function useUsersSource() { engine.getCurrentUser, engine.getCurrentUser ) - const directory = useWorkspaceDirectory() + const directory = useSyncExternalStore( + engine.subscribeDirectory, + engine.getDirectory, + engine.getDirectory + ) const users = useMemo( - () => - workspaceProfiles(state.users, self, directory?.status === 'loading' ? [] : directory?.users), + () => workspaceProfiles(state.users, self, directory), [state.users, self, directory] ) return { state, self, users, engine } diff --git a/client/features/collab/host-state.test.ts b/client/features/collab/host-state.test.ts index e2b7763d..b173e727 100644 --- a/client/features/collab/host-state.test.ts +++ b/client/features/collab/host-state.test.ts @@ -4,7 +4,7 @@ import { colorForId } from '@/lib/collab/colors' import type { UserProfile } from '@/lib/collab/types' import type * as HostStateModule from './host-state' -import type { HostState, HostStateInput, HostApi, WorkspaceDirectory } from './host-state' +import type { HostApi, HostUsers } from './host-state' const PROFILE_KEY = 'moi:collab:dev-profile' const alice = { id: 'alice', name: 'Alice', color: 'emerald' } satisfies UserProfile @@ -16,11 +16,6 @@ const descriptors = new Map( ) let instance = 0 -function readyDirectory(directory: WorkspaceDirectory | undefined) { - if (directory?.status !== 'ready') throw new Error('Expected a ready directory.') - return directory -} - afterEach(() => { for (const [key, descriptor] of descriptors) { if (descriptor) Object.defineProperty(globalThis, key, descriptor) @@ -28,19 +23,21 @@ afterEach(() => { } }) -async function setup( - getHostState?: () => HostStateInput | undefined, - profile?: UserProfile, - beforeInstall?: (state: typeof HostStateModule) => void, - initializeSaved = true -) { - const host: { - moi?: { - collab?: { - getHostState?: () => HostStateInput | undefined - } - } - } = getHostState ? { moi: { collab: { getHostState } } } : {} +type Host = { moi?: { collab?: HostApi; [key: string]: unknown } } +type SetupOptions = { + // A local profile saved by an earlier session. + profile?: UserProfile + beforeInstall?: (state: typeof HostStateModule) => void + initializeSaved?: boolean + host?: Host +} + +async function setup({ + profile, + beforeInstall, + initializeSaved = true, + host = {} +}: SetupOptions = {}) { const saved = new Map(profile ? [[PROFILE_KEY, JSON.stringify(profile)]] : []) Object.defineProperty(globalThis, 'sessionStorage', { configurable: true, @@ -56,29 +53,30 @@ async function setup( beforeInstall?.(collab) collab.installHostApi() if (profile && initializeSaved) collab.initializeLocalUser(true, {}) - // Installation replaces the bootstrap getter with the complete bridge. + // What a host script sees on `window.moi.collab` once moi's bundle has run. const api = host.moi?.collab as HostApi return { ...collab, api, saved } } -test('bridge installation leaves local identity pending until initialization', async () => { - const collab = await setup() +test('installation exposes publishUsers alone and leaves local identity pending until initialization', async () => { + const host: Host = { moi: { kept: 'by the host' } } + const collab = await setup({ host }) + expect(Object.keys(collab.api)).toEqual(['publishUsers']) + expect(host.moi?.kept).toBe('by the host') expect(collab.getCurrentUser()).toBeUndefined() expect(collab.getCurrentUserSource()).toBeUndefined() - expect(collab.getHostState()).toBeUndefined() - expect(collab.getWorkspaceDirectory('a')).toBeUndefined() + expect(collab.getDirectory()).toBeUndefined() expect(collab.saved.has(PROFILE_KEY)).toBe(false) collab.setLocalUser(alice) expect(collab.getCurrentUser()).toEqual(alice) expect(collab.getCurrentUserSource()).toBe('local') - expect(collab.getHostState()).toBeUndefined() - expect(collab.getWorkspaceDirectory('a')).toBeUndefined() + expect(collab.getDirectory()).toBeUndefined() expect(JSON.parse(collab.saved.get(PROFILE_KEY) ?? 'null')).toEqual(alice) }) test('a saved local profile restores after confirmed local initialization', async () => { const legacyProfile = { ...alice, id: 'dev-legacy' } - const collab = await setup(undefined, legacyProfile) + const collab = await setup({ profile: legacyProfile }) expect(collab.getCurrentUser()).toEqual(legacyProfile) expect(collab.getCurrentUserSource()).toBe('local') expect(JSON.parse(collab.saved.get(PROFILE_KEY)!)).toEqual(legacyProfile) @@ -106,11 +104,11 @@ test('startup creates one local profile after configuration and identity checks, collab.setLocalUser({ ...profile, name: ' Ada Lovelace ', color: 'cyan' }) expect(collab.getCurrentUser()).toEqual({ ...profile, name: 'Ada Lovelace', color: 'cyan' }) expect(JSON.parse(collab.saved.get(PROFILE_KEY)!)).toEqual(collab.getCurrentUser()) - expect(collab.getWorkspaceDirectory('a')).toBeUndefined() + expect(collab.getDirectory()).toBeUndefined() }) test('saved local users stay inactive until collaboration is enabled and the proxy check succeeds', async () => { - const collab = await setup(undefined, alice, undefined, false) + const collab = await setup({ profile: alice, initializeSaved: false }) expect(collab.getCurrentUser()).toBeUndefined() collab.initializeLocalUser(false, {}) collab.initializeLocalUser(true, undefined) @@ -124,7 +122,7 @@ test('saved local users stay inactive until collaboration is enabled and the pro test.each([accessed, { provider: 'cloudflare-access' } as const])( 'proxy identity and sign-out prevent local restoration or creation: %j', async proxyUser => { - const collab = await setup(undefined, alice, undefined, false) + const collab = await setup({ profile: alice, initializeSaved: false }) collab.setProxyUserState(proxyUser) collab.initializeLocalUser(true, proxyUser) collab.initializeLocalUser(true, {}) @@ -156,30 +154,46 @@ test('invalid or unavailable local storage still allows a confirmed local user', expect(inaccessible.getCurrentUser()).toBeDefined() }) +// The host script runs after moi's bundle but before startup finishes, so its +// first publish lands ahead of local initialization. test.each([ - ['signed in', (): HostState => ({ currentUser: bob, workspaces: {} }), bob], - ['signed out', (): HostState => ({ workspaces: {} }), undefined], - [ - 'unavailable', - (): HostState => { - throw new Error('Provider unavailable') - }, - undefined - ] -] as const)('an injected provider remains in charge while %s', async (_state, getter, expected) => { - const collab = await setup(getter, alice) - expect(collab.getCurrentUser()).toEqual(expected) - expect(collab.getCurrentUserSource()).toBe('external') - expect(collab.getWorkspaceDirectory('a')).toEqual({ status: 'loading' }) - collab.setLocalUser(alice) - expect(collab.getCurrentUser()).toEqual(expected) - expect(JSON.parse(collab.saved.get(PROFILE_KEY) ?? 'null')).toEqual(alice) - collab.api.setHostState({ currentUser: bob, workspaces: {} }) - expect(collab.getCurrentUser()).toEqual(bob) + ['signed in', { me: 'bob', users: [bob] }, bob], + ['signed out', { users: [] }, undefined], + ['signed out with a null id', { me: null, users: [] }, undefined] +] as const)( + 'a host that published before startup stays in charge while %s', + async (_state, published, expected) => { + const collab = await setup({ profile: alice, initializeSaved: false }) + collab.api.publishUsers(published) + collab.initializeLocalUser(true, {}) + expect(collab.getCurrentUser()).toEqual(expected) + expect(collab.getCurrentUserSource()).toBe('external') + collab.setLocalUser(alice) + expect(collab.getCurrentUser()).toEqual(expected) + expect(JSON.parse(collab.saved.get(PROFILE_KEY) ?? 'null')).toEqual(alice) + collab.api.publishUsers({ me: 'bob', users: [bob] }) + expect(collab.getCurrentUser()).toEqual(bob) + } +) + +test('a host that publishes after startup replaces the local user', async () => { + const collab = await setup({ profile: alice }) + expect(collab.getCurrentUserSource()).toBe('local') + let changes = 0 + collab.subscribeCurrentUserStore(() => changes++) + collab.api.publishUsers({ me: 'bob', users: [alice, bob] }) + expect([collab.getCurrentUser(), collab.getCurrentUserSource()]).toEqual([bob, 'external']) + expect(changes).toBe(1) + // Taking over the source is itself a change, even for an identical profile. + const late = await setup({ profile: alice }) + late.subscribeCurrentUserStore(() => changes++) + late.api.publishUsers({ me: 'alice', users: [alice] }) + expect([late.getCurrentUser(), late.getCurrentUserSource()]).toEqual([alice, 'external']) + expect(changes).toBe(2) }) test('Cloudflare Access replaces a saved local user, locks edits, and ignores repeats', async () => { - const collab = await setup(undefined, alice) + const collab = await setup({ profile: alice }) collab.setProxyUserState({}) expect(collab.getCurrentUser()).toEqual(alice) expect(collab.getCurrentUserSource()).toBe('local') @@ -190,7 +204,7 @@ test('Cloudflare Access replaces a saved local user, locks edits, and ignores re collab.setLocalUser(bob) expect(collab.getCurrentUser()).toEqual(carol) expect(collab.getCurrentUserSource()).toBe('cloudflare-access') - expect(collab.getWorkspaceDirectory('a')).toBeUndefined() + expect(collab.getDirectory()).toBeUndefined() expect(notifications).toBe(1) collab.setProxyUserState({ provider: 'cloudflare-access' }) expect(collab.getCurrentUser()).toBeUndefined() @@ -202,147 +216,95 @@ test('Cloudflare Access replaces a saved local user, locks edits, and ignores re }) test('Access arriving before setup beats a saved local user, and an outer host beats Access', async () => { - const early = await setup(undefined, alice, state => state.setProxyUserState(accessed)) + const early = await setup({ + profile: alice, + beforeInstall: state => state.setProxyUserState(accessed) + }) expect(early.getCurrentUser()).toEqual(carol) - const hosted = await setup( - () => ({ currentUser: bob, workspaces: {} }), - undefined, - state => state.setProxyUserState(accessed) - ) + const hosted = await setup({ beforeInstall: state => state.setProxyUserState(accessed) }) + hosted.api.publishUsers({ me: 'bob', users: [bob] }) hosted.setProxyUserState(accessed) expect([hosted.getCurrentUser(), hosted.getCurrentUserSource()]).toEqual([bob, 'external']) }) -test('preloaded state exposes stable copied current user and directories across later workspace visits', async () => { - const input: HostState = { - currentUser: alice, - workspaces: { a: { status: 'ready', users: [alice, bob] }, b: { status: 'loading' } } - } - let reads = 0 - const collab = await setup(() => { - reads++ - return input - }) - const state = collab.getHostState()! - expect(state).toEqual(input) - expect(collab.getHostState()).toBe(state) - const ready = collab.getWorkspaceDirectory('a') - expect(collab.getWorkspaceDirectory('a')).toBe(ready) - expect(collab.getWorkspaceDirectory('b')).toEqual({ status: 'loading' }) - expect(collab.getWorkspaceDirectory('unknown')).toEqual({ status: 'loading' }) - expect(collab.getWorkspaceDirectory('unknown')).toBe(collab.getWorkspaceDirectory('unknown')) - expect(reads).toBe(1) +test('a published directory is one stable copy, and the viewer is its listed profile', async () => { + const collab = await setup() + const users = [alice, bob] + collab.api.publishUsers({ me: 'alice', users }) + const directory = collab.getDirectory()! + expect(directory).toEqual([alice, bob]) + expect(directory).not.toBe(users) + expect(collab.getDirectory()).toBe(directory) + expect(collab.getCurrentUser()).toBe(directory[0]) }) test('host profiles without colors resolve consistently, while supplied colors win', async () => { - const collab = await setup(() => ({ - currentUser: { id: 'alice', name: 'Alice' }, - workspaces: { - first: { status: 'ready', users: [{ id: 'alice' }, { id: 'bob' }] }, - second: { status: 'ready', users: [{ id: 'alice' }, { id: 'bob' }] } - } - })) + const collab = await setup() + collab.api.publishUsers({ + me: 'alice', + users: [{ id: 'alice', name: 'Alice' }, { id: 'bob' }] + }) const aliceColor = colorForId('alice') const bobColor = colorForId('bob') expect(collab.getCurrentUser()).toEqual({ id: 'alice', name: 'Alice', color: aliceColor }) - expect(readyDirectory(collab.getWorkspaceDirectory('first')).users).toEqual([ + expect(collab.getDirectory()).toEqual([ { id: 'alice', name: 'Alice', color: aliceColor }, { id: 'bob', color: bobColor } ]) - expect(readyDirectory(collab.getWorkspaceDirectory('second')).users).toEqual( - readyDirectory(collab.getWorkspaceDirectory('first')).users - ) - expect(readyDirectory(collab.getWorkspaceDirectory('first')).users?.[0]).toEqual( - collab.getCurrentUser()! - ) - expect(readyDirectory(collab.getWorkspaceDirectory('second')).users?.[0]).toEqual( - collab.getCurrentUser()! - ) - collab.setHostState({ - currentUser: { id: 'alice', color: 'blue' }, - workspaces: { - first: { status: 'ready', users: [{ id: 'alice' }, { id: 'bob', color: 'amber' }] } - } + collab.api.publishUsers({ + me: 'alice', + users: [ + { id: 'alice', color: 'blue' }, + { id: 'bob', color: 'amber' } + ] }) - expect(readyDirectory(collab.getWorkspaceDirectory('first')).users).toEqual([ + expect(collab.getCurrentUser()).toEqual({ id: 'alice', color: 'blue' }) + expect(collab.getDirectory()).toEqual([ { id: 'alice', color: 'blue' }, { id: 'bob', color: 'amber' } ]) }) test('every subscriber sees the complete new state after an atomic replacement', async () => { - const collab = await setup(() => ({ - currentUser: alice, - workspaces: { a: { status: 'ready', users: [alice] } } - })) + const collab = await setup() + collab.api.publishUsers({ me: 'alice', users: [alice] }) const observed: Array<{ current: UserProfile | undefined - a: WorkspaceDirectory | undefined - b: WorkspaceDirectory | undefined - host: HostState | undefined + directory: readonly UserProfile[] | undefined }> = [] const read = () => { - observed.push({ - current: collab.getCurrentUser(), - a: collab.getWorkspaceDirectory('a'), - b: collab.getWorkspaceDirectory('b'), - host: collab.getHostState() - }) + observed.push({ current: collab.getCurrentUser(), directory: collab.getDirectory() }) } - const unsubscribe = [ - collab.subscribeCurrentUserStore(read), - collab.subscribeWorkspaceUsersStore('a', read), - collab.subscribeWorkspaceUsersStore('b', read), - collab.subscribeHostStateStore(read) - ] - collab.setHostState({ - currentUser: bob, - workspaces: { - a: { status: 'ready', users: [bob] }, - b: { status: 'ready', users: [alice, bob] } - } - }) - expect(observed).toHaveLength(4) + const unsubscribe = [collab.subscribeCurrentUserStore(read), collab.subscribeDirectoryStore(read)] + collab.api.publishUsers({ me: 'bob', users: [alice, bob] }) + expect(observed).toHaveLength(2) for (const state of observed) { expect(state.current).toEqual(bob) - expect(state.a).toEqual({ status: 'ready', users: [bob] }) - expect(state.b).toEqual({ status: 'ready', users: [alice, bob] }) - expect(state.host).toBe(collab.getHostState()) + expect(state.directory).toEqual([alice, bob]) } unsubscribe.forEach(stop => stop()) - collab.setHostState({ workspaces: {} }) - expect(observed).toHaveLength(4) + collab.api.publishUsers({ users: [] }) + expect(observed).toHaveLength(2) }) -test('the current user supplies the same own profile in every membership list without adding membership', async () => { +test('a profile change is one publish, and an unchanged viewer stays quiet', async () => { const collab = await setup() - collab.setHostState({ - currentUser: { ...alice, name: 'Alicia', email: 'alice@example.test' }, - workspaces: { - a: { status: 'ready', users: [alice, bob] }, - b: { status: 'ready', users: [{ ...alice, name: 'Different old name' }] }, - c: { status: 'ready', users: [bob] } - } - }) - expect(readyDirectory(collab.getWorkspaceDirectory('a')).users?.[0]).toEqual( - collab.getCurrentUser()! - ) - expect(readyDirectory(collab.getWorkspaceDirectory('b')).users?.[0]).toEqual( - collab.getCurrentUser()! - ) - expect(readyDirectory(collab.getWorkspaceDirectory('a')).users?.[0]?.name).toBe('Alicia') - expect(readyDirectory(collab.getWorkspaceDirectory('c')).users).toEqual([bob]) - collab.setHostState({ - currentUser: { ...alice, name: 'Alice updated again' }, - workspaces: collab.getHostState()!.workspaces - }) - expect(readyDirectory(collab.getWorkspaceDirectory('a')).users?.[0]?.name).toBe( - 'Alice updated again' - ) - expect(readyDirectory(collab.getWorkspaceDirectory('b')).users?.[0]).toEqual( - collab.getCurrentUser()! - ) - expect(readyDirectory(collab.getWorkspaceDirectory('c')).users).toEqual([bob]) + collab.api.publishUsers({ me: 'alice', users: [alice, bob] }) + const viewer = collab.getCurrentUser() + let viewerChanges = 0 + let directoryChanges = 0 + collab.subscribeCurrentUserStore(() => viewerChanges++) + collab.subscribeDirectoryStore(() => directoryChanges++) + // Someone else changed: the directory updates, the viewer's profile object does not. + collab.api.publishUsers({ me: 'alice', users: [{ ...alice }, { ...bob, name: 'Robert' }] }) + expect(collab.getDirectory()).toEqual([alice, { ...bob, name: 'Robert' }]) + expect(collab.getCurrentUser()).toBe(viewer) + expect([viewerChanges, directoryChanges]).toEqual([0, 1]) + // The viewer changed: the same call updates both. + collab.api.publishUsers({ me: 'alice', users: [{ ...alice, name: 'Alicia' }, bob] }) + expect(collab.getCurrentUser()).toEqual({ ...alice, name: 'Alicia' }) + expect(collab.getDirectory()?.[0]).toBe(collab.getCurrentUser()!) + expect([viewerChanges, directoryChanges]).toEqual([1, 2]) }) test('host snapshots preserve nameless current and other users without inventing profile names', async () => { @@ -354,45 +316,33 @@ test('host snapshots preserve nameless current and other users without inventing email: 'other@example.test', avatar: 'https://example.test/avatar' } as const - collab.setHostState({ - currentUser: self, - workspaces: { - a: { - status: 'ready', - users: [ - { ...self, name: 'Old viewer name' }, - other, - { id: 'empty', name: '', color: 'emerald' }, - { id: 'whitespace', name: ' \t ', color: 'blue' } - ] - } - } + collab.api.publishUsers({ + me: 'viewer', + users: [ + self, + other, + { id: 'empty', name: '', color: 'emerald' }, + { id: 'whitespace', name: ' \t ', color: 'blue' } + ] }) expect(collab.getCurrentUser()).toEqual(self) expect(Object.hasOwn(collab.getCurrentUser()!, 'name')).toBe(false) - const users = readyDirectory(collab.getWorkspaceDirectory('a')).users! + const users = collab.getDirectory()! expect(users).toEqual([ self, other, { id: 'empty', color: 'emerald' }, { id: 'whitespace', color: 'blue' } ]) - expect(Object.hasOwn(users[0]!, 'name')).toBe(false) expect(Object.hasOwn(users[1]!, 'name')).toBe(false) - collab.setHostState({ - currentUser: { ...self, name: '' }, - workspaces: collab.getHostState()!.workspaces - }) - expect(collab.getCurrentUser()).toEqual(self) - expect(readyDirectory(collab.getWorkspaceDirectory('a')).users?.[0]).toEqual( - collab.getCurrentUser()! - ) + expect(Object.hasOwn(users[2]!, 'name')).toBe(false) + expect(Object.hasOwn(users[3]!, 'name')).toBe(false) }) test('optional names and colors still reject invalid values, while ids remain required', async () => { const collab = await setup() - collab.setHostState({ currentUser: alice, workspaces: {} }) - const previous = collab.getHostState() + collab.api.publishUsers({ me: 'alice', users: [alice] }) + const previous = collab.getDirectory() for (const profile of [ { color: alice.color }, { ...alice, id: '' }, @@ -405,134 +355,91 @@ test('optional names and colors still reject invalid values, while ids remain re { ...alice, color: 42 } ]) { // The bridge must validate JavaScript callers as well as typed integrations. - expect(() => - collab.setHostState({ currentUser: profile as UserProfile, workspaces: {} }) - ).toThrow() - expect(collab.getHostState()).toBe(previous) + expect(() => collab.api.publishUsers({ users: [profile as UserProfile] })).toThrow() + expect(collab.getDirectory()).toBe(previous) } expect(collab.normalizeUserProfile({ id: 'id', name: '' })).toEqual({ id: 'id', color: colorForId('id') }) + // Ids are trimmed in profiles, so the viewer's id is matched the same way. + collab.api.publishUsers({ me: ' bob ', users: [alice, { ...bob, id: ' bob ' }] }) + expect(collab.getCurrentUser()).toEqual(bob) }) -test('loading, empty, removed workspaces, and removed members remain distinct and authoritative', async () => { - const collab = await setup() - collab.setHostState({ - currentUser: alice, - workspaces: { - loading: { status: 'loading' }, - empty: { status: 'ready', users: [] }, - members: { status: 'ready', users: [alice, bob] } - } - }) - expect(collab.getWorkspaceDirectory('loading')).toEqual({ status: 'loading' }) - expect(collab.getWorkspaceDirectory('empty')).toEqual({ status: 'ready', users: [] }) - collab.setHostState({ - currentUser: alice, - workspaces: { members: { status: 'ready', users: [] } } - }) - expect(collab.getWorkspaceDirectory('members')).toEqual({ status: 'ready', users: [] }) - expect(collab.getWorkspaceDirectory('empty')).toEqual({ status: 'loading' }) -}) - -test('sign-out atomically clears directories and remains authoritative over a saved local user', async () => { - const collab = await setup(undefined, alice) - collab.setHostState({ - currentUser: bob, - workspaces: { a: { status: 'ready', users: [alice, bob] } } - }) - const observed: Array = [] - const unsubscribe = collab.subscribeHostState(state => observed.push(state)) - expect(observed).toEqual([collab.getHostState()]) - collab.setHostState({ workspaces: collab.getHostState()!.workspaces }) +test('removed members are gone, and sign-out shows no one and outranks a saved local user', async () => { + const collab = await setup({ profile: alice }) + collab.api.publishUsers({ me: 'bob', users: [alice, bob] }) + collab.api.publishUsers({ me: 'bob', users: [bob] }) + expect(collab.getDirectory()).toEqual([bob]) + let viewerChanges = 0 + collab.subscribeCurrentUserStore(() => viewerChanges++) + // A host that still lists users after sign-out shows no one all the same. + collab.api.publishUsers({ users: [alice, bob] }) collab.setLocalUser(alice) expect(collab.getCurrentUser()).toBeUndefined() expect(collab.getCurrentUserSource()).toBe('external') - expect(collab.getHostState()).toEqual({ workspaces: {} }) - expect(collab.getWorkspaceDirectory('a')).toEqual({ status: 'loading' }) - expect(observed.at(-1)).toBe(collab.getHostState()) - unsubscribe() - collab.setHostState({ currentUser: bob, workspaces: {} }) - expect(observed).toHaveLength(2) + expect(collab.getDirectory()).toEqual([]) + expect(viewerChanges).toBe(1) + collab.api.publishUsers({ me: 'bob', users: [alice, bob] }) + expect(collab.getCurrentUser()).toEqual(bob) + expect(collab.getDirectory()).toEqual([alice, bob]) }) -test('invalid updates do not partially publish current user, directory, source, or notifications', async () => { - const collab = await setup(undefined, alice) +test('invalid updates do not partially publish the viewer, directory, source, or notifications', async () => { + const collab = await setup({ profile: alice }) const invalidUser = { ...bob, id: '' } - expect(() => - collab.setHostState({ - currentUser: bob, - workspaces: { invalid: { status: 'ready', users: [invalidUser] } } - }) - ).toThrow() + expect(() => collab.api.publishUsers({ me: 'bob', users: [bob, invalidUser] })).toThrow() expect(collab.getCurrentUserSource()).toBe('local') expect(collab.getCurrentUser()).toEqual(alice) - expect(collab.getHostState()).toBeUndefined() - collab.setHostState({ - currentUser: alice, - workspaces: { a: { status: 'ready', users: [alice] } } - }) - const before = collab.getHostState() + expect(collab.getDirectory()).toBeUndefined() + collab.api.publishUsers({ me: 'alice', users: [alice] }) + const before = { viewer: collab.getCurrentUser(), directory: collab.getDirectory() } let notifications = 0 - collab.subscribeHostStateStore(() => notifications++) - const invalidStates: HostState[] = [ - { currentUser: { ...bob, id: '' }, workspaces: {} }, - { - currentUser: bob, - workspaces: { - valid: { status: 'ready', users: [bob] }, - invalid: { status: 'ready', users: [invalidUser] } - } - }, - { currentUser: bob, workspaces: { a: { status: 'ready', users: [alice, alice] } } }, - { currentUser: undefined, workspaces: { invalid: { status: 'ready', users: [invalidUser] } } }, + collab.subscribeCurrentUserStore(() => notifications++) + collab.subscribeDirectoryStore(() => notifications++) + const invalid: unknown[] = [ + { me: 'bob', users: [bob, invalidUser] }, + { me: 'bob', users: [bob, bob] }, + { users: [invalidUser] }, + // The viewer has to be someone in the list. + { me: 'bob', users: [alice] }, + { me: 'bob', users: [] }, + { me: '', users: [alice] }, // A JavaScript host may pass shapes that TypeScript rejects. - { currentUser: bob, workspaces: [] } as unknown as HostState, - { currentUser: bob, workspaces: { a: { status: 'invalid' } } } as unknown as HostState + { me: 7, users: [alice] }, + { me: 'alice' }, + { me: 'alice', users: { alice } }, + null, + 'alice' ] - for (const invalid of invalidStates) { - expect(() => collab.setHostState(invalid)).toThrow() - expect(collab.getHostState()).toBe(before) - expect(collab.getCurrentUser()).toBe(before!.currentUser) + for (const next of invalid) { + expect(() => collab.api.publishUsers(next as HostUsers)).toThrow() + expect(collab.getCurrentUser()).toBe(before.viewer) + expect(collab.getDirectory()).toBe(before.directory) } expect(notifications).toBe(0) }) test.each(['__proto__', 'constructor', 'toString'])( - 'profiles are immutable and %s is safe as a workspace and user id', + 'profiles are immutable and %s is safe as a user id', async id => { const collab = await setup() + const viewer = { ...alice } const user = { ...bob, id } - const users: UserProfile[] = [user] - const state = { - currentUser: { ...alice }, - workspaces: { - [id]: { status: 'ready' as const, users }, - ordinary: { status: 'ready' as const, users: [alice] } - } - } - collab.setHostState(state) - state.currentUser.name = 'Changed outside' + const users: UserProfile[] = [viewer, user] + collab.api.publishUsers({ me: 'alice', users }) + viewer.name = 'Changed outside' user.name = 'Changed outside' - users.push(alice) + users.push({ id: 'late', color: 'blue' }) try { collab.getCurrentUser()!.name = 'Changed through getter' } catch { // A frozen snapshot may throw; either way, the published profile must stay unchanged. } expect(collab.getCurrentUser()?.name).toBe('Alice') - expect(readyDirectory(collab.getWorkspaceDirectory(id)).users).toEqual([{ ...bob, id }]) - expect(readyDirectory(collab.getWorkspaceDirectory('ordinary')).users).toEqual([alice]) - expect(collab.getWorkspaceDirectory('hasOwnProperty')).toEqual({ status: 'loading' }) - collab.setHostState({ - currentUser: collab.getCurrentUser(), - workspaces: { - ...collab.getHostState()?.workspaces, - [id]: { status: 'ready', users: [alice] } - } - }) - expect(readyDirectory(collab.getWorkspaceDirectory(id)).users).toEqual([alice]) - expect(readyDirectory(collab.getWorkspaceDirectory('ordinary')).users).toEqual([alice]) + expect(collab.getDirectory()).toEqual([alice, { ...bob, id }]) + collab.api.publishUsers({ me: id, users: [alice, { ...bob, id }] }) + expect(collab.getCurrentUser()).toEqual({ ...bob, id }) } ) diff --git a/client/features/collab/host-state.ts b/client/features/collab/host-state.ts index bdc111d2..6ca30092 100644 --- a/client/features/collab/host-state.ts +++ b/client/features/collab/host-state.ts @@ -4,34 +4,37 @@ import type { AuthProvider, ProxyUserState, UserProfile } from '@/lib/collab/typ import { createLocalUser } from './local-user' export type UserProfileInput = Omit & { color?: UserProfile['color'] } -export type WorkspaceDirectory = - | { readonly status: 'loading' } - | { readonly status: 'ready'; readonly users: readonly Profile[] } -type State = { - readonly currentUser?: Profile - readonly workspaces: Readonly>> -} -export type HostStateInput = State -export type HostState = State -export type HostApi = { - getHostState: () => HostState | undefined - setHostState: (state: HostStateInput) => void - subscribeHostState: (listener: (state: HostState | undefined) => void) => () => void +// What an outer host publishes: the viewer's id and everyone moi may show. +// `me` must be listed in `users`; leave it out when nobody is signed in. +export type HostUsers = { + me?: string | null + users: readonly UserProfileInput[] } +// `window.moi.collab`. An outer host calls it from a module script placed after +// moi's bundle, once at startup and again whenever a user or the viewer changes. +// A classic inline script runs before the bundle and finds nothing here. +// +// window.moi.collab.publishUsers({ me: 'alex', users: [{ id: 'alex', name: 'Alex' }] }) +export type HostApi = { publishUsers: (next: HostUsers) => void } // Who supplies the current user: local setup, an outer host, or an auth proxy. export type CurrentUserSource = 'local' | 'external' | AuthProvider // Keep the existing key so local profiles survive the startup change. const PROFILE_KEY = 'moi:collab:dev-profile' +const NO_USERS: readonly UserProfile[] = Object.freeze([]) let currentUser: UserProfile | undefined -let installed = false let currentUserSource: CurrentUserSource | undefined +// One list for the whole app. Undefined until a host publishes; from then on it +// is authoritative, including when it is empty. +let directory: readonly UserProfile[] | undefined +let installed = false const listeners = new Set<() => void>() -const workspaceListeners = new Map void>>() -const hostListeners = new Set<() => void>() -const EMPTY_WORKSPACES = Object.freeze({}) -const LOADING_DIRECTORY = Object.freeze({ status: 'loading' as const }) -let hostState: HostState | undefined +const directoryListeners = new Set<() => void>() + +// Normalized profiles list their fields in one order, so text equality is exact. +function sameProfile(a: UserProfile | undefined, b: UserProfile | undefined): boolean { + return JSON.stringify(a) === JSON.stringify(b) +} export function normalizeUserProfile(value: UserProfileInput): UserProfile { if (!value || typeof value.id !== 'string' || !value.id.trim()) @@ -53,100 +56,51 @@ export function normalizeUserProfile(value: UserProfileInput): UserProfile { return Object.freeze(normalized) } -export function normalizeWorkspaceUsers( - users: readonly UserProfileInput[] -): readonly UserProfile[] { - if (!Array.isArray(users)) throw new Error('Workspace users must be an array.') +export function normalizeUsers(users: readonly UserProfileInput[]): readonly UserProfile[] { + if (!Array.isArray(users)) throw new Error('Users must be an array.') const ids = new Set() const snapshot = users.map(user => { const normalized = normalizeUserProfile(user) - if (ids.has(normalized.id)) throw new Error('Workspace users must have unique ids.') + if (ids.has(normalized.id)) throw new Error('Users must have unique ids.') ids.add(normalized.id) return normalized }) return Object.freeze(snapshot) } -function normalizeHostState(value: HostStateInput): HostState { - if (!value || typeof value !== 'object') throw new Error('Invalid host state.') - const current = - value.currentUser === undefined ? undefined : normalizeUserProfile(value.currentUser) - if (!value.workspaces || typeof value.workspaces !== 'object' || Array.isArray(value.workspaces)) - throw new Error('Host workspaces must be a record.') - const entries = Object.entries(value.workspaces).map(([id, directory]) => { - if (!id.trim() || !directory || typeof directory !== 'object') - throw new Error('Invalid workspace directory.') - if (directory.status === 'loading') return [id, LOADING_DIRECTORY] as const - if (directory.status !== 'ready') throw new Error('Invalid workspace directory status.') - const profiles = normalizeWorkspaceUsers(directory.users) - // The viewer has one global profile; membership is still workspace-specific. - const users = Object.freeze( - profiles.map(user => (current && user.id === current.id ? current : user)) - ) - return [id, Object.freeze({ status: 'ready' as const, users })] as const - }) - // Validate everything before discarding directories on sign-out. - return Object.freeze({ - currentUser: current, - workspaces: current ? Object.freeze(Object.fromEntries(entries)) : EMPTY_WORKSPACES - }) -} - -export function getHostState(): HostState | undefined { - return hostState -} - -export function getWorkspaceDirectory(workspaceId: string): WorkspaceDirectory | undefined { - if (!hostState) return undefined - const directory = Object.hasOwn(hostState.workspaces, workspaceId) - ? hostState.workspaces[workspaceId] - : undefined - return directory ?? LOADING_DIRECTORY -} - -export function setHostState(next: HostStateInput): void { - const normalized = normalizeHostState(next) - const previousDirectories = new Map( - [...workspaceListeners.keys()].map(id => [id, getWorkspaceDirectory(id)]) - ) +// Replaces the viewer and the whole directory in one step. The host sends the +// full list every time, so a removal is simply an id that is no longer there. +// Invalid input throws and publishes nothing. +export function publishUsers(next: HostUsers): void { + if (!next || typeof next !== 'object') throw new Error('Users must be published as an object.') + const users = normalizeUsers(next.users) + let user: UserProfile | undefined + if (next.me !== undefined && next.me !== null) { + if (typeof next.me !== 'string') throw new Error('The current user id must be a string.') + const id = next.me.trim() + user = users.find(candidate => candidate.id === id) + if (!user) throw new Error('The current user must be listed in users.') + } + // An unchanged viewer keeps its profile object and stays quiet: every notice + // makes each open workspace resend that profile to its room. + const changed = currentUserSource !== 'external' || !sameProfile(currentUser, user) // Publish every field before notifying any observer, including transport listeners. - hostState = normalized currentUserSource = 'external' - currentUser = normalized.currentUser - listeners.forEach(listener => listener()) - for (const [id, subscriptions] of workspaceListeners) { - if (getWorkspaceDirectory(id) !== previousDirectories.get(id)) - subscriptions.forEach(listener => listener()) - } - hostListeners.forEach(listener => listener()) -} - -export function subscribeHostStateStore(listener: () => void): () => void { - hostListeners.add(listener) - return () => { - hostListeners.delete(listener) - } + if (changed) currentUser = user + // A signed-out viewer sees no one, whatever the host still lists. + directory = user ? users : NO_USERS + if (changed) listeners.forEach(listener => listener()) + directoryListeners.forEach(listener => listener()) } -export function subscribeHostState(listener: (state: HostState | undefined) => void): () => void { - const unsubscribe = subscribeHostStateStore(() => listener(getHostState())) - listener(getHostState()) - return unsubscribe +export function getDirectory(): readonly UserProfile[] | undefined { + return directory } -export function subscribeWorkspaceUsersStore( - workspaceId: string, - listener: () => void -): () => void { - let subscriptions = workspaceListeners.get(workspaceId) - if (!subscriptions) { - subscriptions = new Set() - workspaceListeners.set(workspaceId, subscriptions) - } - subscriptions.add(listener) +export function subscribeDirectoryStore(listener: () => void): () => void { + directoryListeners.add(listener) return () => { - subscriptions.delete(listener) - if (!subscriptions.size) workspaceListeners.delete(workspaceId) + directoryListeners.delete(listener) } } @@ -175,11 +129,7 @@ export function setProxyUserState({ provider, profile }: ProxyUserState): void { currentUser = undefined } else { const normalized = profile === undefined ? undefined : normalizeUserProfile(profile) - if ( - currentUserSource === provider && - JSON.stringify(currentUser) === JSON.stringify(normalized) - ) - return + if (currentUserSource === provider && sameProfile(currentUser, normalized)) return currentUserSource = provider currentUser = normalized } @@ -205,6 +155,7 @@ export function setLocalUser(next: UserProfile): void { } // Called after startup config and proxy identity settle, before React mounts. +// A host that has already published keeps the viewer it chose, signed in or not. export function initializeLocalUser(enabled: boolean, proxyUser: ProxyUserState | undefined): void { if (!enabled || !proxyUser || proxyUser.provider !== undefined || currentUserSource !== undefined) return @@ -220,35 +171,13 @@ export function initializeLocalUser(enabled: boolean, proxyUser: ProxyUserState setLocalUser(createLocalUser()) } -// A host supplies one initial atomic snapshot before loading moi's modules. +// Runs while moi's bundle evaluates, so a host script placed after the bundle +// finds `publishUsers` at once and can publish before the app mounts. export function installHostApi(): void { if (typeof window === 'undefined' || installed) return installed = true - const host = window as Window & { - moi?: { - collab?: { - getHostState?: () => HostStateInput | undefined - } - [key: string]: unknown - } - } - const previous = host.moi?.collab - if (previous?.getHostState) { - try { - setHostState(previous.getHostState() ?? { workspaces: {} }) - } catch { - // An unavailable host remains authoritative; never restore a local profile. - setHostState({ workspaces: {} }) - } - } + const host = window as Window & { moi?: { collab?: HostApi; [key: string]: unknown } } host.moi ??= {} - const api: HostApi = { - getHostState, - setHostState, - subscribeHostState - } + const api: HostApi = { publishUsers } host.moi.collab = api - if (typeof window.dispatchEvent === 'function') { - window.dispatchEvent(new CustomEvent('moi:collab-ready')) - } } diff --git a/client/features/collab/testing/fake-engine.ts b/client/features/collab/testing/fake-engine.ts index 1cb4e285..5761fb3e 100644 --- a/client/features/collab/testing/fake-engine.ts +++ b/client/features/collab/testing/fake-engine.ts @@ -1,8 +1,7 @@ import type { UserProfile, Connection, PresenceRegistration } from '@/lib/collab/types' import { COLLAB_PROTOCOL_VERSION } from '@/lib/collab/protocol' import type { CollabEngineApi } from '../client' -import { normalizeWorkspaceUsers } from '../host-state' -import type { WorkspaceDirectory } from '../host-state' +import { normalizeUsers } from '../host-state' import { CollabStore } from '../store' export type FakeEngineOptions = { @@ -11,32 +10,23 @@ export type FakeEngineOptions = { otherConnections?: Connection[] // Full fixture directory, including users who are offline. users?: readonly UserProfile[] - directoryStatus?: WorkspaceDirectory['status'] } export type FakeEngine = CollabEngineApi & { setOtherConnections: (otherConnections: Connection[]) => void setUsers: (users: readonly UserProfile[] | undefined) => void - setDirectoryStatus: (status: WorkspaceDirectory['status'] | undefined) => void } export function createFakeEngine({ self, page = 'preview', otherConnections = [], - users, - directoryStatus = users === undefined ? undefined : 'ready' + users }: FakeEngineOptions): FakeEngine { const store = new CollabStore() const presence = new Map() const userListeners = new Set<() => void>() - const directory = users === undefined ? undefined : normalizeWorkspaceUsers(users) + let directory = users === undefined ? undefined : normalizeUsers(users) let profiles = directory ?? (self ? [self] : []) - let directorySnapshot: WorkspaceDirectory | undefined = - directoryStatus === 'ready' - ? { status: directoryStatus, users: directory ?? [] } - : directoryStatus === 'loading' - ? { status: directoryStatus } - : undefined let currentOtherConnections = otherConnections let location: Connection['location'] = { page, status: 'active' } const connections = (): Connection[] => [ @@ -89,8 +79,8 @@ export function createFakeEngine({ subscribe: store.subscribe, getCurrentUser: () => self, subscribeCurrentUser: () => () => {}, - getWorkspaceDirectory: () => directorySnapshot, - subscribeWorkspaceUsers: listener => { + getDirectory: () => directory, + subscribeDirectory: listener => { userListeners.add(listener) return () => { userListeners.delete(listener) @@ -109,21 +99,10 @@ export function createFakeEngine({ announce() }, setUsers: next => { - const directory = next === undefined ? undefined : normalizeWorkspaceUsers(next) - directorySnapshot = - directory === undefined ? undefined : { status: 'ready', users: directory } + directory = next === undefined ? undefined : normalizeUsers(next) if (directory) profiles = directory userListeners.forEach(listener => listener()) announce() - }, - setDirectoryStatus: status => { - directorySnapshot = - status === 'ready' - ? { status, users: profiles } - : status === 'loading' - ? { status } - : undefined - userListeners.forEach(listener => listener()) } } } diff --git a/client/features/collab/workspace-users-menu.test.tsx b/client/features/collab/workspace-users-menu.test.tsx index 6961efcb..0c01fe5c 100644 --- a/client/features/collab/workspace-users-menu.test.tsx +++ b/client/features/collab/workspace-users-menu.test.tsx @@ -38,12 +38,11 @@ function describeTab(tab: string) { function renderUsers(options: FakeEngineOptions, list = false) { const engine = createFakeEngine({ page: 'overview', ...options }) const state = engine.getUsersSnapshot() - const directory = engine.getWorkspaceDirectory() const users = summarizeWorkspaceUsers(state.connections, { currentUser: options.self, connectionId: state.connectionId, page: 'overview', - users: directory?.status === 'ready' ? [...directory.users] : [] + users: engine.getDirectory() ?? [] }) return renderToStaticMarkup( @@ -168,19 +167,14 @@ test('self and peers only on the current tab have informational menu rows', () = expect(html).not.toContain(' { - for (const options of [ - { users: [self, active], directoryStatus: 'loading' as const }, - { users: [] } - ]) { - const html = renderUsers({ - self, - otherConnections: [connection(active, 'overview')], - ...options - }) - expect(html).not.toContain('aria-label="Self"') - expect(html).not.toContain('Active colleague') - } +test('an authoritative empty directory does not leak live users into avatars', () => { + const html = renderUsers({ + self, + users: [], + otherConnections: [connection(active, 'overview')] + }) + expect(html).not.toContain('aria-label="Self"') + expect(html).not.toContain('Active colleague') }) test('menu is hidden without a current user', () => { diff --git a/client/features/collab/workspace-users-menu.tsx b/client/features/collab/workspace-users-menu.tsx index b308f380..1d34d56b 100644 --- a/client/features/collab/workspace-users-menu.tsx +++ b/client/features/collab/workspace-users-menu.tsx @@ -19,7 +19,7 @@ import { tabFromPath } from '@/lib/navigation' import { User } from './components/user' import { UserAvatarGroup } from './components/user-avatar-group' import { pageFromPath, useCollabEnabled, useCollabEngine } from './provider' -import { useConnectionState, useWorkspaceUsers, useWorkspaceDirectory } from './hooks' +import { useConnectionState, useWorkspaceUsers } from './hooks' import { summarizeWorkspaceUsers } from './users' import type { WorkspaceUserInfo } from './users' @@ -42,7 +42,6 @@ export function WorkspaceUsersMenu({ const engine = useCollabEngine() const state = useConnectionState() const workspaceUsers = useWorkspaceUsers() - const directory = useWorkspaceDirectory() const currentUser = useSyncExternalStore( engine.subscribeCurrentUser, engine.getCurrentUser, @@ -89,10 +88,7 @@ export function WorkspaceUsersMenu({ onOpenTab(tab) }} /> - {directory?.status === 'loading' && ( -

Loading workspace users…

- )} - {users.length === 0 && directory?.status !== 'loading' && ( + {users.length === 0 && (

No workspace users to show.

)} diff --git a/client/index.tsx b/client/index.tsx index a64e86f6..b60e5800 100644 --- a/client/index.tsx +++ b/client/index.tsx @@ -23,8 +23,12 @@ if (import.meta.hot) { startHmrWatchdog() } +// Installed while this module evaluates, not inside `init`: an outer host's +// script runs right after this bundle and calls `window.moi.collab.publishUsers` +// straight away, before the async startup below has finished. +installHostApi() + export async function init(el: HTMLElement) { - installHostApi() // Startup config and any proxy-verified user load in parallel with the // main chunk, so both are available synchronously from the first render. const [{ mount }, config, proxyUser] = await Promise.all([ diff --git a/workspace/.claude/skills/moi-workspace/references/COLLAB.md b/workspace/.claude/skills/moi-workspace/references/COLLAB.md index 04b48862..87a8016d 100644 --- a/workspace/.claude/skills/moi-workspace/references/COLLAB.md +++ b/workspace/.claude/skills/moi-workspace/references/COLLAB.md @@ -27,8 +27,8 @@ This tracks tab visibility, not idle time. A user on another page can still be a Use `useMe()` when an action needs the current user's identity, such as assigning a task to yourself. Use `useUser(id)` to resolve a stored author or assignee, including an offline -member. Both return `undefined` when the profile cannot be resolved, including while the -workspace directory loads. Handle that before reading profile fields. +member. Both return `undefined` when the profile cannot be resolved. Handle that before +reading profile fields. #### `useWorkspaceUsers` and `usePeers` @@ -39,8 +39,9 @@ Use `usePeers()` to find other connected users on the current page. It excludes user and combines multiple browser tabs into one user. Set `scope: 'workspace'` to include other pages; its status filter accepts `'active'` or `'away'`. -A host-supplied directory is authoritative, including an empty list. Without one, user -lookups fall back to your profile and connected users; they cannot discover offline members. +A host-supplied directory is authoritative, including an empty list, and is the same in +every workspace. Without one, user lookups fall back to your profile and connected users; +they cannot discover offline members. ### Components From 99d4bcfc104de6d125b7f5f4f370ed5ff1cac4cc Mon Sep 17 00:00:00 2001 From: Alexey Taktarov Date: Fri, 9 Oct 2026 16:03:46 +0200 Subject: [PATCH 2/3] Upd: the pull request desc rule. --- .agents/rules/pull-requests.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.agents/rules/pull-requests.md b/.agents/rules/pull-requests.md index 6062f74b..cf91d663 100644 --- a/.agents/rules/pull-requests.md +++ b/.agents/rules/pull-requests.md @@ -4,10 +4,10 @@ alwaysApply: true --- - Lead with 2–3 sentences of what changed and why, describing behavior the reviewer can observe — never a file-by-file or function-by-function inventory; the diff already shows that. +- Language: concise, simple, easy to read by ACTUAL HUMANS. Formatting: less. - Show the artifact right after the intro: the rendered output, a screenshot for UI changes, a wire sample. One example explains more than any amount of prose. - Keep the whole body around 25 lines. Put deep detail in repo docs (`NOTES.md`, rules) or commit messages and link to it instead of duplicating. - Update the description whenever the design pivots mid-PR. The body must describe the final state — a description written at PR-open and never touched becomes misleading. -- Call out the 2–3 risky or surprising spots a reviewer should look at closely, with the reason each is risky. -- End with how the change was verified (test command and count, manual steps). +- Optionally, call out the risky or surprising spots a reviewer should look at closely, with the reason each is risky. - Wrap literal tags like `` in backticks — bare angle-bracket tags are parsed as HTML by GitHub and silently vanish. - Write the title as behavior, sentence case, no trailing period (e.g. `Add moi-context envelope for ambient workspace state`). From 7b533956670719510235401392367f62bd986ac5 Mon Sep 17 00:00:00 2001 From: Alexey Taktarov Date: Fri, 9 Oct 2026 16:06:00 +0200 Subject: [PATCH 3/3] Remove user directory details from the collab applet guide The guide is for agents building applets, who only read users through hooks. Where the user list comes from is nothing an applet can see or act on, so the paragraph about a host-supplied directory goes. Co-Authored-By: Claude Opus 5.5 --- workspace/.claude/skills/moi-workspace/references/COLLAB.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/workspace/.claude/skills/moi-workspace/references/COLLAB.md b/workspace/.claude/skills/moi-workspace/references/COLLAB.md index 87a8016d..fcc2addb 100644 --- a/workspace/.claude/skills/moi-workspace/references/COLLAB.md +++ b/workspace/.claude/skills/moi-workspace/references/COLLAB.md @@ -39,10 +39,6 @@ Use `usePeers()` to find other connected users on the current page. It excludes user and combines multiple browser tabs into one user. Set `scope: 'workspace'` to include other pages; its status filter accepts `'active'` or `'away'`. -A host-supplied directory is authoritative, including an empty list, and is the same in -every workspace. Without one, user lookups fall back to your profile and connected users; -they cannot discover offline members. - ### Components #### `User` and `UserAvatar`