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`). 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..fcc2addb 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,9 +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. Without one, user -lookups fall back to your profile and connected users; they cannot discover offline members. - ### Components #### `User` and `UserAvatar`