diff --git a/AGENTS.md b/AGENTS.md index 09f349344..256f5f9ca 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -225,7 +225,8 @@ as a first language. - `state: true` sends a full-store proposal after each local mutation. Keep transient and high-frequency state in an unsynchronized store. - State, action arguments, and action results must support `structuredClone`. - Keep computed values, query status, runtime clients, controllers, pending promises, and component state outside synchronized state. -- Remote snapshots run local Vue watchers. Never let a watcher on synchronized state write synchronized state or call a synchronized action. +- Remote snapshots run local Vue watchers. A watcher on synchronized state must not write synchronized state directly. +- A watcher can call a synchronized action to enforce a leader-owned invariant. The watcher must await the action. The action must be idempotent because each renderer can observe the same snapshot. - Enforce cross-field invariants inside explicit actions before the state commit. Do not repair replicated state with a watcher. - Every returned function in a setup store is a Pinia action. Use computed values or pure helpers for read-only projections. - List only leader-owned side-effecting actions under `synced.actions`. These actions must be asynchronous, and callers must await them. @@ -233,7 +234,7 @@ as a first language. - Keep synchronization and persistence as separate boundaries. Give persisted synchronized state one explicit persistence owner. - Do not add bidirectional persistence composables or storage-event listeners to synchronized state. Use explicit persistence commands. - Set the leadership mode explicitly for every Electron renderer. Utility and minimal windows must use `follower-only`. -- Add a multi-window regression test for synchronization changes. One remote snapshot must not produce another mutation or action. +- Add a multi-window regression test for synchronization changes. A remote snapshot must not produce a local synchronized-state proposal. If a watcher calls a synchronized action, verify that repeated calls converge without repeated side effects. ### Readability Refactors diff --git a/packages/stage-ui/src/stores/chat/session-store.browser.test.ts b/packages/stage-ui/src/stores/chat/session-store.browser.test.ts new file mode 100644 index 000000000..f139b5fa0 --- /dev/null +++ b/packages/stage-ui/src/stores/chat/session-store.browser.test.ts @@ -0,0 +1,190 @@ +import type { LeadershipMode, SyncedPiniaRuntime } from 'pinia-plugin-synced' + +import type { ChatSessionMeta } from '../../types/chat-session' + +import { createPinia, defineStore, disposePinia, setActivePinia } from 'pinia' +import { createSyncedPiniaPlugin } from 'pinia-plugin-synced' +import { afterEach, describe, expect, it, vi } from 'vitest' +import { createApp, ref } from 'vue' + +const useTestAuthStore = defineStore('auth', () => { + const userId = ref('local') + const token = ref(null) + return { userId, token } +}, { + synced: { state: true }, +}) + +const useTestAiriCardStore = defineStore('airi-card', () => { + const activeCardId = ref('default') + const systemPrompt = ref('') + return { activeCardId, systemPrompt } +}) + +vi.doMock('../auth', () => { + return { + useAuthStore: useTestAuthStore, + } +}) + +vi.doMock('../modules/airi-card', () => { + return { + useAiriCardStore: useTestAiriCardStore, + } +}) + +vi.mock('../../database/repos/chat-sessions.repo', () => ({ + chatSessionsRepo: { + addTombstone: vi.fn().mockResolvedValue(undefined), + deleteSession: vi.fn().mockResolvedValue(undefined), + dequeueOutbox: vi.fn().mockResolvedValue(undefined), + dropOutboxForSession: vi.fn().mockResolvedValue(undefined), + enqueueOutbox: vi.fn().mockResolvedValue(undefined), + getIndex: vi.fn().mockResolvedValue(null), + getOutbox: vi.fn().mockResolvedValue([]), + getSession: vi.fn().mockResolvedValue(null), + getTombstones: vi.fn().mockResolvedValue([]), + removeTombstones: vi.fn().mockResolvedValue(undefined), + saveIndex: vi.fn().mockResolvedValue(undefined), + saveSession: vi.fn().mockResolvedValue(undefined), + updateOutboxEntries: vi.fn().mockResolvedValue(undefined), + }, +})) + +vi.mock('../../libs/analytics', () => ({ + captureAnalyticsEvent: vi.fn(), +})) + +vi.mock('../../libs/auth-fetch', () => ({ + authedFetch: vi.fn(), +})) + +vi.mock('../../libs/server', () => ({ + SERVER_URL: 'http://test', +})) + +vi.mock('../../libs/chat-sync', () => ({ + applyCreateActions: vi.fn().mockResolvedValue([]), + createCloudChatMapper: () => ({ + deleteChat: vi.fn().mockResolvedValue(undefined), + listChats: vi.fn().mockResolvedValue([]), + }), + createChatWsClient: () => ({ + connect: vi.fn(), + destroy: vi.fn(), + disconnect: vi.fn(), + onNewMessages: () => () => {}, + onStatusChange: () => () => {}, + pullMessages: vi.fn().mockResolvedValue({ messages: [], seq: 0 }), + sendMessages: vi.fn().mockResolvedValue({ ok: true }), + status: () => 'idle', + }), + extractMessageText: () => '', + isCloudSyncableMessage: () => false, + mergeCloudMessagesIntoLocal: () => ({ dirty: false, messages: [], maxSeq: 0 }), + reconcileLocalAndRemote: () => ({ adopt: [], claim: [], create: [] }), +})) + +const { useChatSessionStore } = await import('./session-store') + +const syncedContexts: Array<{ + pinia: ReturnType + runtime: SyncedPiniaRuntime +}> = [] + +function createSyncedContext(namespace: string, leadership: LeadershipMode) { + const pinia = createPinia() + const runtime = createSyncedPiniaPlugin({ + callTimeout: 1000, + leadership, + namespace, + }) + pinia.use(runtime.plugin) + createApp({}).use(pinia) + syncedContexts.push({ pinia, runtime }) + return { pinia, runtime } +} + +afterEach(() => { + for (const context of syncedContexts.splice(0)) { + context.runtime.dispose() + disposePinia(context.pinia) + } +}) + +describe('chat session synchronization', () => { + it('keeps the leader chat snapshot when new followers receive the auth identity', async () => { + // ROOT CAUSE: + // + // A new settings renderer received the synchronized auth identity after + // its chat-session store was created. Its local userId watcher cleared the + // synchronized chat state and proposed that empty snapshot to the leader. + // + // The follower routes its observed auth transition to the synchronized + // identity action. The leader keeps its matching snapshot unchanged. + const namespace = `chat-session:${crypto.randomUUID()}` + const leaderContext = createSyncedContext(namespace, 'leader-only') + await vi.waitFor(() => expect(leaderContext.runtime.isLeader()).toBe(true)) + + setActivePinia(leaderContext.pinia) + const leaderAuthStore = useTestAuthStore() + leaderAuthStore.userId = 'cloud-user' + const leaderChatStore = useChatSessionStore() + + const session: ChatSessionMeta = { + sessionId: 'session-a', + userId: 'cloud-user', + characterId: 'default', + createdAt: 1, + updatedAt: 1, + } + leaderChatStore.$patch({ + index: { + userId: 'cloud-user', + characters: { + default: { + activeSessionId: 'session-a', + sessions: { 'session-a': session }, + }, + }, + }, + sessionMessages: { + 'session-a': [{ id: 'message-a', role: 'user', content: 'Keep this message' }], + }, + sessionMetas: { 'session-a': session }, + }) + + let leaderIdentityActions = 0 + let leaderMutations = 0 + leaderChatStore.$onAction(({ name }) => { + if (name === 'activateCurrentUser') + leaderIdentityActions++ + }) + leaderChatStore.$subscribe(() => leaderMutations++) + + const followerContext = createSyncedContext(namespace, 'follower-only') + setActivePinia(followerContext.pinia) + const followerChatStore = useChatSessionStore() + const followerAuthStore = useTestAuthStore() + await vi.waitFor(() => expect(followerContext.runtime.getLeaderId()).toBe(leaderContext.runtime.participantId)) + await vi.waitFor(() => expect(followerAuthStore.userId).toBe('cloud-user')) + await vi.waitFor(() => expect(followerChatStore.sessionMessages['session-a']).toHaveLength(1)) + + const secondFollowerContext = createSyncedContext(namespace, 'follower-only') + setActivePinia(secondFollowerContext.pinia) + const secondFollowerChatStore = useChatSessionStore() + const secondFollowerAuthStore = useTestAuthStore() + await vi.waitFor(() => expect(secondFollowerContext.runtime.getLeaderId()).toBe(leaderContext.runtime.participantId)) + await vi.waitFor(() => expect(secondFollowerAuthStore.userId).toBe('cloud-user')) + await vi.waitFor(() => expect(secondFollowerChatStore.sessionMessages['session-a']).toHaveLength(1)) + await Promise.resolve() + + expect(leaderChatStore.sessionMessages['session-a']?.[0]?.id).toBe('message-a') + expect(followerChatStore.sessionMessages['session-a']?.[0]?.id).toBe('message-a') + expect(leaderChatStore.index?.userId).toBe('cloud-user') + expect(followerChatStore.index?.userId).toBe('cloud-user') + expect(secondFollowerChatStore.sessionMessages['session-a']?.[0]?.id).toBe('message-a') + expect(leaderIdentityActions).toBe(2) + expect(leaderMutations).toBe(0) + }) +}) diff --git a/packages/stage-ui/src/stores/chat/session-store.test.ts b/packages/stage-ui/src/stores/chat/session-store.test.ts index 71780f183..12e1c4235 100644 --- a/packages/stage-ui/src/stores/chat/session-store.test.ts +++ b/packages/stage-ui/src/stores/chat/session-store.test.ts @@ -147,15 +147,8 @@ describe('chat-session-store · user swap during in-flight ensureActiveSessionFo // ROOT CAUSE: // // ensureActiveSessionForCharacter caches `ensureActivePromise` for singleflight - // and the IIFE captures `currentUserId` at start. When `userId` flips A → B - // mid-flight: - // 1. The userId watcher calls clearInMemoryState (resets sessionMetas / - // index / activeSessionId), but does NOT reset `ensureActivePromise`. - // 2. A's IIFE eventually resumes after its awaited IDB read completes and - // writes A's session record back into the now-empty B state — leak. - // 3. Any subsequent ensureActiveSessionForCharacter call (e.g. from the - // [userId, activeCardId] watcher) returns A's stale promise instead of - // starting a fresh hydrate for B — B silently sees no sessions. + // and the IIFE captures `currentUserId` at start. An explicit A → B identity + // transition must invalidate A's in-flight read before it hydrates B. // // We fix this by: // - bumping an `ensureActiveEpoch` and nulling `ensureActivePromise` in @@ -163,8 +156,7 @@ describe('chat-session-store · user swap during in-flight ensureActiveSessionFo // - re-checking the captured epoch after each await inside the IIFE, // - re-checking `sessionMetas[sessionId]` inside `loadSession` so the // post-IDB write does not resurrect cleared state, - // - triggering a fresh hydrate from the userId watcher itself so the new - // user actually loads. + // - hydrating the new identity only through `activateCurrentUser`. it('runs a fresh hydrate for the new user and discards the stale write from the old user', async () => { const aSessionMeta: ChatSessionMeta = { sessionId: 'sess-A', @@ -232,7 +224,7 @@ describe('chat-session-store · user swap during in-flight ensureActiveSessionFo expect(getSessionMock).toHaveBeenCalledWith('sess-A') expect(resolveASessionGet).toBeDefined() - // Auth swap mid-flight. + // The synchronized auth state changes while A's session read is in flight. userIdRef.value = 'B' await nextTick() await flushMicrotasks() @@ -243,8 +235,8 @@ describe('chat-session-store · user swap during in-flight ensureActiveSessionFo await initPromise.catch(() => {}) await flushMicrotasks() - // B's hydrate must have fired — without the fix, the [userId, activeCardId] - // watcher returned the stale A promise and B never loaded. + // B's hydrate must have fired. Without the fix, the stale A promise blocks + // the identity action and B never loads. expect(getIndexMock).toHaveBeenCalledWith('B') expect(store.sessionMetas['sess-B']).toBeDefined() @@ -719,6 +711,57 @@ describe('chat-session-store · active card prompt edits', () => { }) describe('chat-session-store · synchronized data actions', () => { + it('keeps synchronized session data when a follower receives authenticated user state', async () => { + // ROOT CAUSE: + // + // A new settings window received the synchronized auth user after its + // chat-session store was created. The userId watcher then cleared the + // synchronized session state in that follower. pinia-plugin-synced sent + // the empty full-state proposal to the leader and removed chat messages + // from every window. + // + // The watcher routes the identity transition to an idempotent synchronized + // action. The action keeps state that already belongs to the current user. + const session: ChatSessionMeta = { + sessionId: 'session-a', + userId: 'cloud-user', + characterId: 'default', + createdAt: 1, + updatedAt: 1, + } + const store = useChatSessionStore() + store.setCloudSyncOwnership(false) + store.applyRemoteSnapshot({ + activeSessionId: 'session-a', + sessionMessages: { + 'session-a': [{ id: 'message-a', role: 'user', content: 'Keep this message' }], + }, + sessionMetas: { 'session-a': session }, + index: { + userId: 'cloud-user', + characters: { + default: { + activeSessionId: 'session-a', + sessions: { 'session-a': session }, + }, + }, + }, + }) + await nextTick() + await flushMicrotasks() + const messageIdsBeforeAuthHydration = store.sessionMessages['session-a'].map(message => message.id) + const metaBeforeAuthHydration = { ...store.sessionMetas['session-a'] } + + userIdRef.value = 'cloud-user' + await nextTick() + await flushMicrotasks() + + expect(store.sessionMessages['session-a'].map(message => message.id)).toEqual(messageIdsBeforeAuthHydration) + expect(store.sessionMetas['session-a']).toEqual(metaBeforeAuthHydration) + expect(store.activeSessionId).toBe('session-a') + expect(store.index?.userId).toBe('cloud-user') + }) + // https://github.com/moeru-ai/airi/pull/2086#discussion_r3755711151 it('keeps cloud synchronization in the elected leader for Issue #2085', async () => { // ROOT CAUSE: @@ -734,7 +777,7 @@ describe('chat-session-store · synchronized data actions', () => { expect(connectCloudWsMock).not.toHaveBeenCalled() store.setCloudSyncOwnership(true) - expect(connectCloudWsMock).toHaveBeenCalledTimes(1) + await vi.waitFor(() => expect(connectCloudWsMock).toHaveBeenCalledTimes(1)) }) // https://github.com/moeru-ai/airi/pull/2086#discussion_r3743242525 @@ -752,6 +795,7 @@ describe('chat-session-store · synchronized data actions', () => { updatedAt: 1, } const store = useChatSessionStore() + store.setCloudSyncOwnership(false) store.$patch({ sessionMessages: { 'session-b': [{ id: 'system', role: 'system', content: 'prompt' }] }, sessionMetas: { 'session-b': session }, @@ -773,6 +817,7 @@ describe('chat-session-store · synchronized data actions', () => { expect(store.activeSessionId).toBe('session-b') expect(store.isReady).toBe(true) + expect(getSessionMock).not.toHaveBeenCalled() }) // https://github.com/moeru-ai/airi/pull/2086#discussion_r3743242529 diff --git a/packages/stage-ui/src/stores/chat/session-store.ts b/packages/stage-ui/src/stores/chat/session-store.ts index 64f2ac761..ca45d76d9 100644 --- a/packages/stage-ui/src/stores/chat/session-store.ts +++ b/packages/stage-ui/src/stores/chat/session-store.ts @@ -1072,11 +1072,38 @@ export const useChatSessionStore = defineStore('chat-session', () => { index.value = null activeSessionId.value = '' cloudSyncReady.value = false - // outbox count reflects the prior user; reset to 0 — the next user's - // refreshOutboxPendingCount fires from initialize() once they hydrate. + // The outbox count belongs to the prior user. The identity action refreshes + // this count after it hydrates the next user. outboxPendingCount.value = 0 } + function sessionStateMatchesCurrentUser() { + const currentUserId = getCurrentUserId() + const hasOnlyCurrentUserSessions = Object.values(sessionMetas.value) + .every(meta => meta.userId === currentUserId) + return index.value?.userId === currentUserId && hasOnlyCurrentUserSessions + } + + /** + * Replaces session state after the synchronized auth identity changes. + * The synchronization plugin routes this action to the elected renderer. + */ + async function activateCurrentUser() { + if (sessionStateMatchesCurrentUser()) { + ensureCloudWsClient() + return + } + + teardownCloudWsClient() + clearInMemoryState() + if (!ready.value && !initializing.value) + return + + await ensureActiveSessionForCharacter() + await refreshOutboxPendingCount() + ensureCloudWsClient() + } + /** * Refresh the reactive `outboxPendingCount` from IDB. Called after every * enqueue / dequeue / drain so UI banners stay in sync with reality. @@ -1291,12 +1318,17 @@ export const useChatSessionStore = defineStore('chat-session', () => { } initializing.value = true initializePromise = (async () => { - await ensureActiveSessionForCharacter() + if (ownsCloudSync) + await ensureActiveSessionForCharacter() + else + selectWindowSessionFromIndex() + ready.value = true // Surface any outbox left over from a previous session (closed tab // mid-send) before the WS even opens. The drain itself runs after // reconcile completes, but the count is observable immediately. - await refreshOutboxPendingCount() + if (ownsCloudSync) + await refreshOutboxPendingCount() if (ownsCloudSync) ensureCloudWsClient() })() @@ -1322,6 +1354,17 @@ export const useChatSessionStore = defineStore('chat-session', () => { || !!Object.values(index.value?.characters ?? {}).some(character => character.sessions[sessionId]) } + /** Selects the persisted session for this window without changing synchronized session data. */ + function selectWindowSessionFromIndex() { + const currentUserId = getCurrentUserId() + if (!index.value || index.value.userId !== currentUserId) { + activeSessionId.value = '' + return + } + + activeSessionId.value = getCharacterIndex(getCurrentCharacterId())?.activeSessionId ?? '' + } + const messages = computed({ get: () => { if (!activeSessionId.value) { @@ -1552,41 +1595,34 @@ export const useChatSessionStore = defineStore('chat-session', () => { // every follower would fan one deletion out into several empty chats. }) - watch([userId, activeCardId], () => { + watch([activeCardId, index], () => { if (!ready.value) return + + if (!ownsCloudSync) { + selectWindowSessionFromIndex() + return + } + void ensureActiveSessionForCharacter() }) + // Each renderer observes the synchronized identity. Route the transition to + // the leader so followers never write synchronized chat state directly. + watch(userId, async () => { + try { + await useChatSessionStore().activateCurrentUser() + } + catch (error) { + console.error('[chat-session] Failed to activate the current user:', error) + } + }) + // Keep the active conversation aligned with edits to the active card. The // active session id is included because card switching resolves the target // session asynchronously after the card prompt itself has already changed. watch([systemPrompt, activeSessionId], refreshActiveSessionSystemMessage) - // Auth toggles drive cloud WS lifecycle independently of activeCardId so - // a card swap inside a single session does not bounce the socket. The - // critical invariant: when the auth user changes, every piece of in-memory - // state from the previous user must be cleared BEFORE the new user's WS - // and reconcile fire. Otherwise the previous user's sessionMetas would - // leak into the new user's drawer, exports, and (worst) into the cloud - // reconcile's `localOwnedMetas` snapshot. - watch(userId, (next) => { - teardownCloudWsClient() - clearInMemoryState() - if (ownsCloudSync && next && next !== 'local') { - ensureCloudWsClient() - } - // Rehydrate for the new user. We trigger here (instead of relying on the - // `[userId, activeCardId]` watcher) because that watcher gates on - // `ready.value` — if the swap happens while initialize() is still - // awaiting the prior user's hydrate, the gated trigger is dropped and - // the new user silently sees no sessions. `clearInMemoryState` already - // bumped `ensureActiveEpoch` and freed the singleflight slot, so this - // call starts a fresh IIFE that runs alongside (and is unaffected by) - // any in-flight stale hydrate. - void ensureActiveSessionForCharacter() - }) - return { isReady, initialize, @@ -1623,6 +1659,7 @@ export const useChatSessionStore = defineStore('chat-session', () => { loadSession, refreshSession, deleteSession, + activateCurrentUser, setCloudSyncOwnership, @@ -1632,7 +1669,7 @@ export const useChatSessionStore = defineStore('chat-session', () => { } }, { synced: { - actions: ['createSession', 'deleteMessage', 'importSessions', 'loadSession', 'refreshSession'], + actions: ['activateCurrentUser', 'createSession', 'deleteMessage', 'importSessions', 'loadSession', 'refreshSession'], state: true, }, })