Skip to content

Commit 29341bf

Browse files
j15zj15z
committed
fix(copilot): serialize resource view updates
1 parent 0a0b72a commit 29341bf

14 files changed

Lines changed: 606 additions & 228 deletions

File tree

‎apps/sim/app/api/copilot/chat/resources/route.ts‎

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import type { MothershipResourceUpdate } from '@/lib/copilot/resources/types'
2121
import {
2222
canonicalizeDesktopSessionResource,
2323
mergeChatResource,
24+
reorderStoredChatResources,
2425
sanitizeChatResources,
2526
} from '@/lib/copilot/resources/types'
2627
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -140,16 +141,8 @@ export const PATCH = withRouteHandler(async (req: NextRequest) => {
140141
const existing = sanitizeChatResources(
141142
Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : []
142143
)
143-
// The client echoes the tabs it holds; anything it does not carry (a view
144-
// pin, a path) is taken from the stored entry rather than dropped.
145-
const existingByKey = new Map(existing.map((r) => [`${r.type}:${r.id}`, r]))
146-
const canonicalOrder = sanitizeChatResources(newOrder).map((r) =>
147-
mergeChatResource(existingByKey.get(`${r.type}:${r.id}`), r)
148-
)
149-
const existingKeys = new Set(existingByKey.keys())
150-
const newKeys = new Set(canonicalOrder.map((r) => `${r.type}:${r.id}`))
151-
152-
if (existingKeys.size !== newKeys.size || ![...existingKeys].every((k) => newKeys.has(k))) {
144+
const canonicalOrder = reorderStoredChatResources(existing, newOrder)
145+
if (!canonicalOrder) {
153146
return createBadRequestResponse('Reordered resources must match existing resources')
154147
}
155148

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts‎

Lines changed: 111 additions & 164 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import {
6666
} from '@/lib/copilot/request/session/file-preview-session-contract'
6767
import type { StreamBatchEvent } from '@/lib/copilot/request/session/types'
6868
import { canDisplayResource } from '@/lib/copilot/resources/availability'
69+
import { ResourcePersistenceQueue } from '@/lib/copilot/resources/client-persistence-queue'
6970
import {
7071
BROWSER_SESSION_RESOURCE_ID,
7172
isAddressableResource,
@@ -1384,9 +1385,27 @@ export function useChat(
13841385
* make the tabs disappear for the desktop app too.
13851386
*/
13861387
const undisplayableResourcesRef = useRef<MothershipResource[]>([])
1387-
const pendingPersistResourceKeysRef = useRef<Set<string>>(new Set())
1388-
const pendingClearViewIdKeysRef = useRef<Set<string>>(new Set())
1389-
const inFlightResourceAddsRef = useRef<Map<string, Promise<unknown>>>(new Map())
1388+
const resourcePersistenceQueueRef = useRef<ResourcePersistenceQueue | null>(null)
1389+
if (!resourcePersistenceQueueRef.current) {
1390+
resourcePersistenceQueueRef.current = new ResourcePersistenceQueue({
1391+
persist: (chatId, update) => {
1392+
const { clearViewId, ...resource } = update
1393+
return requestJson(addMothershipChatResourceContract, {
1394+
body: {
1395+
chatId,
1396+
resource,
1397+
...(clearViewId === true ? { clearViewId: true as const } : {}),
1398+
},
1399+
})
1400+
},
1401+
onError: (error) => {
1402+
logger.warn('Failed to persist resource; will retry on next hydration', error)
1403+
},
1404+
})
1405+
}
1406+
const resourcePersistenceQueue = resourcePersistenceQueueRef.current
1407+
const pendingPersistResourceKeysRef = useRef(resourcePersistenceQueue.pendingKeys)
1408+
const inFlightResourceAddsRef = useRef(resourcePersistenceQueue.inFlight)
13901409
const reorderNeededAfterFlushRef = useRef(false)
13911410

13921411
// Derive the effective active resource ID for rendering without writing a
@@ -1644,9 +1663,7 @@ export function useChat(
16441663
// Pending view pins belong to the chat whose stream issued them.
16451664
useTableViewPinStore.getState().reset()
16461665
undisplayableResourcesRef.current = []
1647-
pendingPersistResourceKeysRef.current.clear()
1648-
pendingClearViewIdKeysRef.current.clear()
1649-
inFlightResourceAddsRef.current.clear()
1666+
resourcePersistenceQueue.clear()
16501667
reorderNeededAfterFlushRef.current = false
16511668
resetEphemeralPreviewState()
16521669
// Editing binds to this hook's composer — release it before rotating chatKey.
@@ -1674,56 +1691,29 @@ export function useChat(
16741691
workspaceId,
16751692
])
16761693

1677-
const flushPendingResources = useCallback(async (chatId: string) => {
1678-
const pendingKeys = pendingPersistResourceKeysRef.current
1679-
if (pendingKeys.size === 0) return
1680-
const flushPromises: Array<Promise<unknown>> = []
1681-
for (const resource of resourcesRef.current) {
1682-
if (resource.id === 'streaming-file') continue
1683-
const key = `${resource.type}:${resource.id}`
1684-
if (!pendingKeys.has(key)) continue
1685-
pendingKeys.delete(key)
1686-
const shouldClearViewId = pendingClearViewIdKeysRef.current.has(key)
1687-
const promise = requestJson(addMothershipChatResourceContract, {
1688-
body: {
1689-
chatId,
1690-
resource,
1691-
...(shouldClearViewId ? { clearViewId: true as const } : {}),
1692-
},
1694+
const flushPendingResources = useCallback(
1695+
async (chatId: string) => {
1696+
if (pendingPersistResourceKeysRef.current.size === 0) return
1697+
await resourcePersistenceQueue.flush(chatId)
1698+
if (!reorderNeededAfterFlushRef.current) return
1699+
reorderNeededAfterFlushRef.current = false
1700+
const localOrder = [
1701+
...resourcesRef.current.filter(
1702+
(r) =>
1703+
r.id !== 'streaming-file' &&
1704+
!pendingPersistResourceKeysRef.current.has(`${r.type}:${r.id}`)
1705+
),
1706+
...undisplayableResourcesRef.current,
1707+
]
1708+
if (localOrder.length === 0) return
1709+
requestJson(reorderMothershipChatResourcesContract, {
1710+
body: { chatId, resources: localOrder },
1711+
}).catch((err) => {
1712+
logger.warn('Failed to sync resource order after flush', err)
16931713
})
1694-
.then((result) => {
1695-
if (shouldClearViewId) pendingClearViewIdKeysRef.current.delete(key)
1696-
return result
1697-
})
1698-
.catch((err) => {
1699-
pendingPersistResourceKeysRef.current.add(key)
1700-
logger.warn('Failed to flush pending resource; will retry on next hydration', err)
1701-
})
1702-
.finally(() => {
1703-
inFlightResourceAddsRef.current.delete(key)
1704-
})
1705-
inFlightResourceAddsRef.current.set(key, promise)
1706-
flushPromises.push(promise)
1707-
}
1708-
if (flushPromises.length === 0) return
1709-
await Promise.allSettled(flushPromises)
1710-
if (!reorderNeededAfterFlushRef.current) return
1711-
reorderNeededAfterFlushRef.current = false
1712-
const localOrder = [
1713-
...resourcesRef.current.filter(
1714-
(r) =>
1715-
r.id !== 'streaming-file' &&
1716-
!pendingPersistResourceKeysRef.current.has(`${r.type}:${r.id}`)
1717-
),
1718-
...undisplayableResourcesRef.current,
1719-
]
1720-
if (localOrder.length === 0) return
1721-
requestJson(reorderMothershipChatResourcesContract, {
1722-
body: { chatId, resources: localOrder },
1723-
}).catch((err) => {
1724-
logger.warn('Failed to sync resource order after flush', err)
1725-
})
1726-
}, [])
1714+
},
1715+
[resourcePersistenceQueue]
1716+
)
17271717

17281718
const adoptResolvedChatId = useCallback(
17291719
(chatId: string, options?: { replaceHomeHistory?: boolean; invalidateList?: boolean }) => {
@@ -1811,113 +1801,76 @@ export function useChat(
18111801
const source = chatHistory?.messages.map(toDisplayMessage) ?? pendingMessages
18121802
return source.map((m) => restoreRevealedSimKeysForMessage(m, revealedSimKeysRef.current))
18131803
}, [chatHistory, pendingMessages])
1814-
const addResource = useCallback((resourceUpdate: MothershipResourceUpdate): boolean => {
1815-
// The single fan-in for tab creation, so the invariant lives here.
1816-
if (!isAddressableResource(resourceUpdate)) {
1817-
logger.warn('Ignored a resource with no id', {
1818-
type: resourceUpdate.type,
1819-
title: resourceUpdate.title,
1820-
})
1821-
return false
1822-
}
1823-
const existing = resourcesRef.current.find(
1824-
(r) => r.type === resourceUpdate.type && r.id === resourceUpdate.id
1825-
)
1826-
const resource = mergeChatResource(existing, resourceUpdate)
1827-
if (existing && resource === existing) {
1828-
return false
1829-
}
1830-
1831-
setResources((prev) => {
1832-
const current = prev.find((r) => r.type === resource.type && r.id === resource.id)
1833-
if (!current) return [...prev, resource]
1834-
const merged = mergeChatResource(current, resourceUpdate)
1835-
return merged === current
1836-
? prev
1837-
: prev.map((r) => (r.type === resource.type && r.id === resource.id ? merged : r))
1838-
})
1839-
// Synthetic result/preview panels are in-memory only. The browser tab
1840-
// metadata is persisted even though its live page remains desktop-owned.
1841-
if (isEphemeralResource(resource)) {
1842-
return true
1843-
}
1804+
const addResource = useCallback(
1805+
(resourceUpdate: MothershipResourceUpdate): boolean => {
1806+
// The single fan-in for tab creation, so the invariant lives here.
1807+
if (!isAddressableResource(resourceUpdate)) {
1808+
logger.warn('Ignored a resource with no id', {
1809+
type: resourceUpdate.type,
1810+
title: resourceUpdate.title,
1811+
})
1812+
return false
1813+
}
1814+
const existing = resourcesRef.current.find(
1815+
(r) => r.type === resourceUpdate.type && r.id === resourceUpdate.id
1816+
)
1817+
const resource = mergeChatResource(existing, resourceUpdate)
1818+
if (existing && resource === existing) {
1819+
return false
1820+
}
18441821

1845-
const persistChatId = chatIdRef.current ?? selectedChatIdRef.current
1846-
const key = `${resource.type}:${resource.id}`
1847-
const shouldClearViewId = resourceUpdate.clearViewId === true
1848-
if (shouldClearViewId) {
1849-
pendingClearViewIdKeysRef.current.add(key)
1850-
} else if (resourceUpdate.viewId !== undefined) {
1851-
pendingClearViewIdKeysRef.current.delete(key)
1852-
}
1853-
// `resourcesRef` is written during render, so adds of the same resource in
1854-
// one tick all read the pre-render list and all pass the check above. State
1855-
// converges (the updater is idempotent) but each fired its own POST — 5-6
1856-
// per resource in production.
1857-
const alreadyPersisting =
1858-
inFlightResourceAddsRef.current.has(key) || pendingPersistResourceKeysRef.current.has(key)
1859-
if (alreadyPersisting) {
1860-
pendingPersistResourceKeysRef.current.add(key)
1861-
return existing === undefined
1862-
}
1863-
if (persistChatId) {
1864-
const promise = requestJson(addMothershipChatResourceContract, {
1865-
body: {
1866-
chatId: persistChatId,
1867-
resource,
1868-
...(shouldClearViewId ? { clearViewId: true as const } : {}),
1869-
},
1822+
setResources((prev) => {
1823+
const current = prev.find((r) => r.type === resource.type && r.id === resource.id)
1824+
if (!current) return [...prev, resource]
1825+
const merged = mergeChatResource(current, resourceUpdate)
1826+
return merged === current
1827+
? prev
1828+
: prev.map((r) => (r.type === resource.type && r.id === resource.id ? merged : r))
18701829
})
1871-
.then((result) => {
1872-
if (shouldClearViewId) pendingClearViewIdKeysRef.current.delete(key)
1873-
return result
1874-
})
1875-
.catch((err) => {
1876-
pendingPersistResourceKeysRef.current.add(key)
1877-
logger.warn('Failed to persist resource; will retry on next hydration', err)
1878-
})
1879-
.finally(() => {
1880-
inFlightResourceAddsRef.current.delete(key)
1881-
})
1882-
inFlightResourceAddsRef.current.set(key, promise)
1883-
} else {
1884-
pendingPersistResourceKeysRef.current.add(key)
1885-
}
1886-
return existing === undefined
1887-
}, [])
1830+
// Synthetic result/preview panels are in-memory only. The browser tab
1831+
// metadata is persisted even though its live page remains desktop-owned.
1832+
if (isEphemeralResource(resource)) {
1833+
return true
1834+
}
18881835

1889-
const removeResource = useCallback((resourceType: MothershipResourceType, resourceId: string) => {
1890-
setResources((prev) => prev.filter((r) => !(r.type === resourceType && r.id === resourceId)))
1891-
setActiveResourceId((prev) => (prev === resourceId ? null : prev))
1836+
const persistChatId = chatIdRef.current ?? selectedChatIdRef.current
1837+
resourcePersistenceQueue.enqueue(resourceUpdate, persistChatId, existing)
1838+
return existing === undefined
1839+
},
1840+
[resourcePersistenceQueue]
1841+
)
18921842

1893-
// Ephemeral panels were never persisted; nothing to delete server-side.
1894-
if (isEphemeralResource({ type: resourceType, id: resourceId, title: '' })) return
1843+
const removeResource = useCallback(
1844+
(resourceType: MothershipResourceType, resourceId: string) => {
1845+
setResources((prev) => prev.filter((r) => !(r.type === resourceType && r.id === resourceId)))
1846+
setActiveResourceId((prev) => (prev === resourceId ? null : prev))
18951847

1896-
const key = `${resourceType}:${resourceId}`
1897-
pendingClearViewIdKeysRef.current.delete(key)
1898-
const wasPending = pendingPersistResourceKeysRef.current.delete(key)
1899-
const inFlightAdd = inFlightResourceAddsRef.current.get(key)
1900-
if (wasPending && !inFlightAdd) return
1848+
// Ephemeral panels were never persisted; nothing to delete server-side.
1849+
if (isEphemeralResource({ type: resourceType, id: resourceId, title: '' })) return
19011850

1902-
const persistChatId = chatIdRef.current ?? selectedChatIdRef.current
1903-
if (!persistChatId) return
1904-
const fireDelete = () => {
1905-
requestJson(removeMothershipChatResourceContract, {
1906-
body: { chatId: persistChatId, resourceType, resourceId },
1907-
}).catch((err) => {
1908-
logger.warn('Failed to persist resource removal', err)
1909-
})
1910-
}
1911-
if (inFlightAdd) {
1912-
// Drop the entry now, not when the add settles: an add being deleted must
1913-
// not suppress a fresh add of the same resource. The chained delete keeps
1914-
// its own reference to the promise.
1915-
inFlightResourceAddsRef.current.delete(key)
1916-
inFlightAdd.finally(fireDelete)
1917-
} else {
1918-
fireDelete()
1919-
}
1920-
}, [])
1851+
const { inFlight: inFlightAdd, wasPending } = resourcePersistenceQueue.remove(
1852+
resourceType,
1853+
resourceId
1854+
)
1855+
if (wasPending && !inFlightAdd) return
1856+
1857+
const persistChatId = chatIdRef.current ?? selectedChatIdRef.current
1858+
if (!persistChatId) return
1859+
const fireDelete = () => {
1860+
requestJson(removeMothershipChatResourceContract, {
1861+
body: { chatId: persistChatId, resourceType, resourceId },
1862+
}).catch((err) => {
1863+
logger.warn('Failed to persist resource removal', err)
1864+
})
1865+
}
1866+
if (inFlightAdd) {
1867+
inFlightAdd.finally(fireDelete)
1868+
} else {
1869+
fireDelete()
1870+
}
1871+
},
1872+
[resourcePersistenceQueue]
1873+
)
19211874

19221875
/**
19231876
* Drops hydrated workflow tabs whose workflow no longer exists, so an old
@@ -2280,11 +2233,7 @@ export function useChat(
22802233
const streamOwnerId = chatIdRef.current
22812234
const pendingTurn = activeTurnRef.current
22822235
const pendingStreamId = streamIdRef.current ?? pendingTurn?.userMessageId
2283-
const pendingResources = resourcesRef.current.filter(
2284-
(resource) =>
2285-
!isEphemeralResource(resource) &&
2286-
pendingPersistResourceKeysRef.current.has(`${resource.type}:${resource.id}`)
2287-
)
2236+
const pendingResources = resourcePersistenceQueue.getPendingUpdates()
22882237
const navigatedToDifferentChat =
22892238
sendingRef.current &&
22902239
initialChatId !== streamOwnerId &&
@@ -2382,9 +2331,7 @@ export function useChat(
23822331
setResources([])
23832332
setActiveResourceId(null)
23842333
useTableViewPinStore.getState().reset()
2385-
pendingPersistResourceKeysRef.current.clear()
2386-
pendingClearViewIdKeysRef.current.clear()
2387-
inFlightResourceAddsRef.current.clear()
2334+
resourcePersistenceQueue.clear()
23882335
reorderNeededAfterFlushRef.current = false
23892336
resetEphemeralPreviewState()
23902337
// Rotate the bucket key; the previous chat's queue stays in the store.

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/view-state.test.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,20 @@ describe('resolveTableViewPinTransition', () => {
9292
resolveTableViewPinTransition('view-pinned', 'view-created', 'view-pinned', 'view-created')
9393
).toEqual({ nextViewId: null, pendingCreatedViewId: 'view-created' })
9494
})
95+
96+
it('replaces a different active URL even if the pin was applied previously', () => {
97+
expect(resolveTableViewPinTransition('view-user', 'view-pinned', 'view-pinned', null)).toEqual({
98+
nextViewId: 'view-pinned',
99+
pendingCreatedViewId: null,
100+
})
101+
})
102+
103+
it('suppresses a redundant URL update while the applied view has no URL selection', () => {
104+
expect(resolveTableViewPinTransition(null, 'view-pinned', 'view-pinned', null)).toEqual({
105+
nextViewId: null,
106+
pendingCreatedViewId: null,
107+
})
108+
})
95109
})
96110

97111
describe('shouldApplyTableViewRevision', () => {

‎apps/sim/app/workspace/[workspaceId]/tables/[tableId]/view-state.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export function resolveTableViewPinTransition(
6464
pinnedViewId: string,
6565
pendingCreatedViewId: string | null
6666
): TableViewPinTransition {
67-
if (activeViewId === pinnedViewId || appliedViewId === pinnedViewId) {
67+
if (activeViewId === pinnedViewId || (activeViewId === null && appliedViewId === pinnedViewId)) {
6868
return { nextViewId: null, pendingCreatedViewId }
6969
}
7070
return { nextViewId: pinnedViewId, pendingCreatedViewId: null }

‎apps/sim/lib/api/contracts/copilot.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ const copilotChatResourceItemSchema = z.object({
102102

103103
export const addCopilotChatResourceBodySchema = z
104104
.object({
105-
chatId: z.string(),
105+
chatId: requiredFieldSchema('chatId cannot be empty'),
106106
resource: copilotChatResourceItemSchema,
107107
clearViewId: z.literal(true).optional(),
108108
})

0 commit comments

Comments
 (0)