mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-24 11:10:25 +02:00
Merge pull request #81 from multica-ai/fix/ai-response-display
fix: resolve race condition causing AI responses not to display
This commit is contained in:
@@ -75,6 +75,76 @@ export interface AppActions {
|
||||
setSessionModel: (modelId: ModelId) => Promise<void>
|
||||
}
|
||||
|
||||
function mergeSessionUpdates(
|
||||
existing: StoredSessionUpdate[],
|
||||
incoming: StoredSessionUpdate[]
|
||||
): StoredSessionUpdate[] {
|
||||
if (incoming.length === 0) {
|
||||
return existing
|
||||
}
|
||||
|
||||
const merged: Array<StoredSessionUpdate | null> = []
|
||||
const keyToIndex = new Map<string, number>()
|
||||
const payloadToKey = new Map<string, string>()
|
||||
|
||||
const buildPayloadKey = (item: StoredSessionUpdate): string => {
|
||||
try {
|
||||
return JSON.stringify(item.update)
|
||||
} catch {
|
||||
return `unstringifiable:${item.timestamp}`
|
||||
}
|
||||
}
|
||||
|
||||
const addItem = (item: StoredSessionUpdate): void => {
|
||||
const payloadKey = buildPayloadKey(item)
|
||||
if (item.sequenceNumber !== undefined) {
|
||||
const seqKey = `seq:${item.sequenceNumber}`
|
||||
const existingPayloadKey = payloadToKey.get(payloadKey)
|
||||
if (existingPayloadKey?.startsWith('payload:')) {
|
||||
const existingIndex = keyToIndex.get(existingPayloadKey)
|
||||
if (existingIndex !== undefined) {
|
||||
merged[existingIndex] = null
|
||||
}
|
||||
keyToIndex.delete(existingPayloadKey)
|
||||
}
|
||||
|
||||
const existingIndex = keyToIndex.get(seqKey)
|
||||
if (existingIndex !== undefined) {
|
||||
merged[existingIndex] = item
|
||||
} else {
|
||||
merged.push(item)
|
||||
keyToIndex.set(seqKey, merged.length - 1)
|
||||
}
|
||||
payloadToKey.set(payloadKey, seqKey)
|
||||
return
|
||||
}
|
||||
|
||||
const payloadKeyWithPrefix = `payload:${payloadKey}`
|
||||
const existingPayloadKey = payloadToKey.get(payloadKey)
|
||||
if (existingPayloadKey?.startsWith('seq:')) {
|
||||
return
|
||||
}
|
||||
|
||||
const existingIndex = keyToIndex.get(payloadKeyWithPrefix)
|
||||
if (existingIndex !== undefined) {
|
||||
merged[existingIndex] = item
|
||||
} else {
|
||||
merged.push(item)
|
||||
keyToIndex.set(payloadKeyWithPrefix, merged.length - 1)
|
||||
}
|
||||
payloadToKey.set(payloadKey, payloadKeyWithPrefix)
|
||||
}
|
||||
|
||||
for (const item of existing) {
|
||||
addItem(item)
|
||||
}
|
||||
for (const item of incoming) {
|
||||
addItem(item)
|
||||
}
|
||||
|
||||
return merged.filter((item): item is StoredSessionUpdate => item !== null)
|
||||
}
|
||||
|
||||
export function useApp(): AppState & AppActions {
|
||||
// Track pending session selection to handle rapid switching
|
||||
const pendingSessionRef = useRef<string | null>(null)
|
||||
@@ -88,6 +158,14 @@ export function useApp(): AppState & AppActions {
|
||||
const [sessions, setSessions] = useState<MulticaSession[]>([])
|
||||
const [currentSession, setCurrentSession] = useState<MulticaSession | null>(null)
|
||||
const [sessionUpdates, setSessionUpdates] = useState<StoredSessionUpdate[]>([])
|
||||
|
||||
// Helper to update current session with synchronous ref update
|
||||
// This ensures currentSessionIdRef is always up-to-date before any async operations
|
||||
// Critical for avoiding race conditions where messages arrive before useEffect updates the ref
|
||||
const updateCurrentSession = useCallback((session: MulticaSession | null) => {
|
||||
currentSessionIdRef.current = session?.id ?? null // Sync update FIRST
|
||||
setCurrentSession(session)
|
||||
}, [])
|
||||
const [runningSessionsStatus, setRunningSessionsStatus] = useState<RunningSessionsStatus>({
|
||||
runningSessions: 0,
|
||||
sessionIds: [],
|
||||
@@ -135,14 +213,14 @@ export function useApp(): AppState & AppActions {
|
||||
'agentSessionId:',
|
||||
updatedSession.agentSessionId
|
||||
)
|
||||
setCurrentSession(updatedSession)
|
||||
updateCurrentSession(updatedSession)
|
||||
}
|
||||
})
|
||||
|
||||
return () => {
|
||||
unsubSessionMeta()
|
||||
}
|
||||
}, [currentSessionId])
|
||||
}, [currentSessionId, updateCurrentSession])
|
||||
|
||||
// Subscribe to file system change events (runs once on mount)
|
||||
useEffect(() => {
|
||||
@@ -303,7 +381,7 @@ export function useApp(): AppState & AppActions {
|
||||
|
||||
// Only update if state changed to avoid unnecessary re-renders
|
||||
if (session.directoryExists !== currentSession.directoryExists) {
|
||||
setCurrentSession(session)
|
||||
updateCurrentSession(session)
|
||||
// Also refresh sidebar list to update directory status indicators
|
||||
const list = await window.electronAPI.listSessions()
|
||||
setSessions(list)
|
||||
@@ -311,7 +389,7 @@ export function useApp(): AppState & AppActions {
|
||||
} catch (err) {
|
||||
console.error('Failed to validate session directory:', err)
|
||||
}
|
||||
}, [currentSession])
|
||||
}, [currentSession, updateCurrentSession])
|
||||
|
||||
// Subscribe to app focus event to validate directory existence
|
||||
useEffect(() => {
|
||||
@@ -333,7 +411,7 @@ export function useApp(): AppState & AppActions {
|
||||
try {
|
||||
setIsInitializing(true)
|
||||
const session = await window.electronAPI.createSession(cwd, agentId)
|
||||
setCurrentSession(session)
|
||||
updateCurrentSession(session)
|
||||
setSessionUpdates([])
|
||||
await loadSessions()
|
||||
await loadRunningStatus()
|
||||
@@ -345,7 +423,7 @@ export function useApp(): AppState & AppActions {
|
||||
setIsInitializing(false)
|
||||
}
|
||||
},
|
||||
[loadSessions, loadRunningStatus, loadSessionModeModel]
|
||||
[loadSessions, loadRunningStatus, loadSessionModeModel, updateCurrentSession]
|
||||
)
|
||||
|
||||
const selectSession = useCallback(
|
||||
@@ -366,7 +444,7 @@ export function useApp(): AppState & AppActions {
|
||||
}
|
||||
|
||||
// Update both states together - React will batch them into one render
|
||||
setCurrentSession(session)
|
||||
updateCurrentSession(session)
|
||||
setSessionUpdates(data?.updates ?? [])
|
||||
|
||||
// Start agent if not already running
|
||||
@@ -376,7 +454,7 @@ export function useApp(): AppState & AppActions {
|
||||
try {
|
||||
const updatedSession = await window.electronAPI.startSessionAgent(sessionId)
|
||||
if (pendingSessionRef.current !== sessionId) return
|
||||
setCurrentSession(updatedSession)
|
||||
updateCurrentSession(updatedSession)
|
||||
} finally {
|
||||
setIsInitializing(false)
|
||||
}
|
||||
@@ -384,6 +462,19 @@ export function useApp(): AppState & AppActions {
|
||||
|
||||
// Agent now guaranteed running, load mode/model
|
||||
await loadSessionModeModel(sessionId)
|
||||
|
||||
// Reload session updates to catch any messages that arrived during async operations
|
||||
// This is critical because messages may have arrived via IPC while we were:
|
||||
// 1. Loading initial history
|
||||
// 2. Starting the agent
|
||||
// 3. Loading mode/model state
|
||||
// Without this reload, those messages would be lost (overwritten by initial setSessionUpdates)
|
||||
if (pendingSessionRef.current === sessionId) {
|
||||
const freshData = await window.electronAPI.getSession(sessionId)
|
||||
if (pendingSessionRef.current === sessionId && freshData?.updates) {
|
||||
setSessionUpdates((prev) => mergeSessionUpdates(prev, freshData.updates))
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
// Only show error if this is still the pending session
|
||||
if (pendingSessionRef.current === sessionId) {
|
||||
@@ -391,7 +482,7 @@ export function useApp(): AppState & AppActions {
|
||||
}
|
||||
}
|
||||
},
|
||||
[loadSessionModeModel]
|
||||
[loadSessionModeModel, updateCurrentSession]
|
||||
)
|
||||
|
||||
const deleteSession = useCallback(
|
||||
@@ -400,7 +491,7 @@ export function useApp(): AppState & AppActions {
|
||||
await window.electronAPI.deleteSession(sessionId)
|
||||
useDraftStore.getState().clearDraft(sessionId)
|
||||
if (currentSession?.id === sessionId) {
|
||||
setCurrentSession(null)
|
||||
updateCurrentSession(null)
|
||||
setSessionUpdates([])
|
||||
}
|
||||
await loadSessions()
|
||||
@@ -409,16 +500,16 @@ export function useApp(): AppState & AppActions {
|
||||
toast.error(`Failed to delete session: ${getErrorMessage(err)}`)
|
||||
}
|
||||
},
|
||||
[currentSession, loadSessions, loadRunningStatus]
|
||||
[currentSession, loadSessions, loadRunningStatus, updateCurrentSession]
|
||||
)
|
||||
|
||||
const clearCurrentSession = useCallback(() => {
|
||||
setCurrentSession(null)
|
||||
updateCurrentSession(null)
|
||||
setSessionUpdates([])
|
||||
setSessionModeState(null)
|
||||
setSessionModelState(null)
|
||||
useCommandStore.getState().clearCommands()
|
||||
}, [])
|
||||
}, [updateCurrentSession])
|
||||
|
||||
const sendPrompt = useCallback(
|
||||
async (content: MessageContent) => {
|
||||
@@ -502,7 +593,7 @@ export function useApp(): AppState & AppActions {
|
||||
currentSession.id,
|
||||
newAgentId
|
||||
)
|
||||
setCurrentSession(updatedSession)
|
||||
updateCurrentSession(updatedSession)
|
||||
await loadRunningStatus()
|
||||
// Reload mode/model state for the new agent
|
||||
await loadSessionModeModel(currentSession.id)
|
||||
@@ -513,7 +604,7 @@ export function useApp(): AppState & AppActions {
|
||||
setIsSwitchingAgent(false)
|
||||
}
|
||||
},
|
||||
[currentSession, loadRunningStatus, loadSessionModeModel]
|
||||
[currentSession, loadRunningStatus, loadSessionModeModel, updateCurrentSession]
|
||||
)
|
||||
|
||||
// Mode/Model actions
|
||||
|
||||
@@ -6,14 +6,25 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { useApp } from '../../../../src/renderer/src/hooks/useApp'
|
||||
import { useDraftStore } from '../../../../src/renderer/src/stores/draftStore'
|
||||
import type { StoredSessionUpdate, MulticaSession } from '../../../../src/shared/types'
|
||||
|
||||
type AppHandle = {
|
||||
deleteSession: (sessionId: string) => Promise<void>
|
||||
selectSession: (sessionId: string) => Promise<void>
|
||||
getSessionUpdates: () => StoredSessionUpdate[]
|
||||
}
|
||||
|
||||
const AppHarness = React.forwardRef<AppHandle>((_, ref) => {
|
||||
const { deleteSession } = useApp()
|
||||
useImperativeHandle(ref, () => ({ deleteSession }), [deleteSession])
|
||||
const { deleteSession, selectSession, sessionUpdates } = useApp()
|
||||
useImperativeHandle(
|
||||
ref,
|
||||
() => ({
|
||||
deleteSession,
|
||||
selectSession,
|
||||
getSessionUpdates: () => sessionUpdates
|
||||
}),
|
||||
[deleteSession, selectSession, sessionUpdates]
|
||||
)
|
||||
return null
|
||||
})
|
||||
|
||||
@@ -37,6 +48,9 @@ describe('useApp deleteSession', () => {
|
||||
sessionIds: [],
|
||||
processingSessionIds: []
|
||||
}),
|
||||
loadSession: vi.fn().mockResolvedValue(null),
|
||||
getSession: vi.fn().mockResolvedValue({ session: null, updates: [] }),
|
||||
startSessionAgent: vi.fn().mockResolvedValue(null),
|
||||
getSessionModes: vi.fn().mockResolvedValue(null),
|
||||
getSessionModels: vi.fn().mockResolvedValue(null),
|
||||
getSessionCommands: vi.fn().mockResolvedValue([]),
|
||||
@@ -82,4 +96,236 @@ describe('useApp deleteSession', () => {
|
||||
const { drafts } = useDraftStore.getState()
|
||||
expect(drafts['session-a']).toBeUndefined()
|
||||
})
|
||||
|
||||
it('merges reloaded updates with in-memory updates', async () => {
|
||||
let agentMessageHandler:
|
||||
| ((message: {
|
||||
sessionId: string
|
||||
multicaSessionId: string
|
||||
sequenceNumber?: number
|
||||
update: { sessionUpdate: string; content?: { type: string; text: string } }
|
||||
done: boolean
|
||||
}) => void)
|
||||
| undefined
|
||||
|
||||
const session: MulticaSession = {
|
||||
id: 'session-a',
|
||||
agentSessionId: 'agent-1',
|
||||
agentId: 'codex',
|
||||
workingDirectory: '/tmp',
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
updatedAt: '2024-01-01T00:00:00.000Z',
|
||||
status: 'active',
|
||||
messageCount: 0
|
||||
}
|
||||
|
||||
const update1: StoredSessionUpdate = {
|
||||
timestamp: '2024-01-01T00:00:01.000Z',
|
||||
sequenceNumber: 1,
|
||||
update: {
|
||||
sessionId: 'agent-1',
|
||||
update: {
|
||||
sessionUpdate: 'agent_message_chunk',
|
||||
content: { type: 'text', text: 'hello' }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const update2: StoredSessionUpdate = {
|
||||
timestamp: '2024-01-01T00:00:02.000Z',
|
||||
sequenceNumber: 2,
|
||||
update: {
|
||||
sessionId: 'agent-1',
|
||||
update: {
|
||||
sessionUpdate: 'agent_message_chunk',
|
||||
content: { type: 'text', text: 'world' }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const modesDeferred = createDeferred<null>()
|
||||
const modelsDeferred = createDeferred<null>()
|
||||
const commandsDeferred = createDeferred<unknown[]>()
|
||||
|
||||
const electronAPI = createElectronApiMock()
|
||||
electronAPI.onAgentMessage.mockImplementation((cb) => {
|
||||
agentMessageHandler = cb
|
||||
return () => {}
|
||||
})
|
||||
const loadSessionDeferred = createDeferred<MulticaSession>()
|
||||
const getSessionDeferred = createDeferred<{
|
||||
session: MulticaSession
|
||||
updates: StoredSessionUpdate[]
|
||||
}>()
|
||||
electronAPI.loadSession.mockImplementation(() => loadSessionDeferred.promise)
|
||||
electronAPI.getAgentStatus.mockResolvedValue({
|
||||
runningSessions: 1,
|
||||
sessionIds: ['session-a'],
|
||||
processingSessionIds: []
|
||||
})
|
||||
electronAPI.getSession
|
||||
.mockImplementationOnce(() => getSessionDeferred.promise)
|
||||
.mockResolvedValueOnce({ session, updates: [update1, update2] })
|
||||
electronAPI.getSessionModes.mockImplementation(() => modesDeferred.promise)
|
||||
electronAPI.getSessionModels.mockImplementation(() => modelsDeferred.promise)
|
||||
electronAPI.getSessionCommands.mockImplementation(() => commandsDeferred.promise)
|
||||
;(window as unknown as { electronAPI: typeof electronAPI }).electronAPI = electronAPI
|
||||
|
||||
const ref = React.createRef<AppHandle>()
|
||||
|
||||
await act(async () => {
|
||||
root.render(<AppHarness ref={ref} />)
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
const selectPromise = ref.current?.selectSession('session-a')
|
||||
loadSessionDeferred.resolve(session)
|
||||
getSessionDeferred.resolve({ session, updates: [update1] })
|
||||
await waitForSessionUpdates(ref, 1)
|
||||
agentMessageHandler?.({
|
||||
sessionId: 'agent-1',
|
||||
multicaSessionId: 'session-a',
|
||||
sequenceNumber: 2,
|
||||
update: {
|
||||
sessionUpdate: 'agent_message_chunk',
|
||||
content: { type: 'text', text: 'world' }
|
||||
},
|
||||
done: false
|
||||
})
|
||||
modesDeferred.resolve(null)
|
||||
modelsDeferred.resolve(null)
|
||||
commandsDeferred.resolve([])
|
||||
await selectPromise
|
||||
})
|
||||
|
||||
const updates = ref.current?.getSessionUpdates() ?? []
|
||||
const sequenceNumbers = updates
|
||||
.map((update) => update.sequenceNumber)
|
||||
.filter((seq): seq is number => seq !== undefined)
|
||||
expect(sequenceNumbers).toEqual([1, 2])
|
||||
})
|
||||
|
||||
it('keeps in-memory updates when refreshed updates are empty', async () => {
|
||||
let agentMessageHandler:
|
||||
| ((message: {
|
||||
sessionId: string
|
||||
multicaSessionId: string
|
||||
sequenceNumber?: number
|
||||
update: { sessionUpdate: string; content?: { type: string; text: string } }
|
||||
done: boolean
|
||||
}) => void)
|
||||
| undefined
|
||||
|
||||
const session: MulticaSession = {
|
||||
id: 'session-a',
|
||||
agentSessionId: 'agent-1',
|
||||
agentId: 'codex',
|
||||
workingDirectory: '/tmp',
|
||||
createdAt: '2024-01-01T00:00:00.000Z',
|
||||
updatedAt: '2024-01-01T00:00:00.000Z',
|
||||
status: 'active',
|
||||
messageCount: 0
|
||||
}
|
||||
|
||||
const update1: StoredSessionUpdate = {
|
||||
timestamp: '2024-01-01T00:00:01.000Z',
|
||||
sequenceNumber: 1,
|
||||
update: {
|
||||
sessionId: 'agent-1',
|
||||
update: {
|
||||
sessionUpdate: 'agent_message_chunk',
|
||||
content: { type: 'text', text: 'hello' }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
const modesDeferred = createDeferred<null>()
|
||||
const modelsDeferred = createDeferred<null>()
|
||||
const commandsDeferred = createDeferred<unknown[]>()
|
||||
|
||||
const electronAPI = createElectronApiMock()
|
||||
electronAPI.onAgentMessage.mockImplementation((cb) => {
|
||||
agentMessageHandler = cb
|
||||
return () => {}
|
||||
})
|
||||
const loadSessionDeferred = createDeferred<MulticaSession>()
|
||||
const getSessionDeferred = createDeferred<{
|
||||
session: MulticaSession
|
||||
updates: StoredSessionUpdate[]
|
||||
}>()
|
||||
electronAPI.loadSession.mockImplementation(() => loadSessionDeferred.promise)
|
||||
electronAPI.getAgentStatus.mockResolvedValue({
|
||||
runningSessions: 1,
|
||||
sessionIds: ['session-a'],
|
||||
processingSessionIds: []
|
||||
})
|
||||
electronAPI.getSession
|
||||
.mockImplementationOnce(() => getSessionDeferred.promise)
|
||||
.mockResolvedValueOnce({ session, updates: [] })
|
||||
electronAPI.getSessionModes.mockImplementation(() => modesDeferred.promise)
|
||||
electronAPI.getSessionModels.mockImplementation(() => modelsDeferred.promise)
|
||||
electronAPI.getSessionCommands.mockImplementation(() => commandsDeferred.promise)
|
||||
;(window as unknown as { electronAPI: typeof electronAPI }).electronAPI = electronAPI
|
||||
|
||||
const ref = React.createRef<AppHandle>()
|
||||
|
||||
await act(async () => {
|
||||
root.render(<AppHarness ref={ref} />)
|
||||
})
|
||||
|
||||
await act(async () => {
|
||||
const selectPromise = ref.current?.selectSession('session-a')
|
||||
loadSessionDeferred.resolve(session)
|
||||
getSessionDeferred.resolve({ session, updates: [update1] })
|
||||
await waitForSessionUpdates(ref, 1)
|
||||
agentMessageHandler?.({
|
||||
sessionId: 'agent-1',
|
||||
multicaSessionId: 'session-a',
|
||||
sequenceNumber: 2,
|
||||
update: {
|
||||
sessionUpdate: 'agent_message_chunk',
|
||||
content: { type: 'text', text: 'world' }
|
||||
},
|
||||
done: false
|
||||
})
|
||||
modesDeferred.resolve(null)
|
||||
modelsDeferred.resolve(null)
|
||||
commandsDeferred.resolve([])
|
||||
await selectPromise
|
||||
})
|
||||
|
||||
const updates = ref.current?.getSessionUpdates() ?? []
|
||||
const sequenceNumbers = updates
|
||||
.map((update) => update.sequenceNumber)
|
||||
.filter((seq): seq is number => seq !== undefined)
|
||||
expect(sequenceNumbers).toEqual([1, 2])
|
||||
})
|
||||
})
|
||||
|
||||
function createDeferred<T>(): {
|
||||
promise: Promise<T>
|
||||
resolve: (value: T) => void
|
||||
reject: (error: unknown) => void
|
||||
} {
|
||||
let resolve: (value: T) => void = () => {}
|
||||
let reject: (error: unknown) => void = () => {}
|
||||
const promise = new Promise<T>((res, rej) => {
|
||||
resolve = res
|
||||
reject = rej
|
||||
})
|
||||
return { promise, resolve, reject }
|
||||
}
|
||||
|
||||
async function waitForSessionUpdates(
|
||||
ref: React.RefObject<AppHandle>,
|
||||
expectedLength: number
|
||||
): Promise<void> {
|
||||
for (let attempt = 0; attempt < 5; attempt += 1) {
|
||||
if ((ref.current?.getSessionUpdates().length ?? 0) >= expectedLength) {
|
||||
return
|
||||
}
|
||||
await act(async () => {
|
||||
await Promise.resolve()
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user