Skip to content

Commit 3e6eb75

Browse files
committed
fix(copilot): persist table view pin updates
1 parent f2fc678 commit 3e6eb75

21 files changed

Lines changed: 336 additions & 37 deletions

File tree

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
createUnauthorizedResponse,
1818
} from '@/lib/copilot/request/http'
1919
import type { ChatResource } from '@/lib/copilot/resources/persistence'
20+
import type { MothershipResourceUpdate } from '@/lib/copilot/resources/types'
2021
import {
2122
canonicalizeDesktopSessionResource,
2223
mergeChatResource,
@@ -43,8 +44,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
4344
}
4445
)
4546
if (!parsed.success) return parsed.response
46-
const { chatId, resource: requestedResource } = parsed.data.body
47+
const { chatId, resource: requestedResource, clearViewId } = parsed.data.body
4748
const resource = canonicalizeDesktopSessionResource(requestedResource)
49+
const resourceUpdate: MothershipResourceUpdate =
50+
clearViewId === true ? { ...resource, clearViewId: true } : resource
4851

4952
// Ephemeral UI tab (client does not POST this; guard for old clients / bugs).
5053
if (resource.id === 'streaming-file') {
@@ -74,8 +77,10 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
7477
const prev = existing.find((r) => `${r.type}:${r.id}` === key)
7578

7679
const merged: ChatResource[] = prev
77-
? existing.map((r) => (`${r.type}:${r.id}` === key ? mergeChatResource(r, resource) : r))
78-
: [...existing, resource]
80+
? existing.map((r) =>
81+
`${r.type}:${r.id}` === key ? mergeChatResource(r, resourceUpdate) : r
82+
)
83+
: [...existing, mergeChatResource(undefined, resourceUpdate)]
7984

8085
await db
8186
.update(copilotChats)

apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.test.ts

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,11 @@ describe('handleResourceEvent removal', () => {
108108
})
109109
})
110110

111-
function tableUpsertEvent(id: string, viewId?: string): PersistedStreamEventEnvelope {
111+
function tableUpsertEvent(
112+
id: string,
113+
viewId?: string,
114+
clearViewId?: true
115+
): PersistedStreamEventEnvelope {
112116
return {
113117
type: 'resource',
114118
v: 1,
@@ -117,7 +121,13 @@ function tableUpsertEvent(id: string, viewId?: string): PersistedStreamEventEnve
117121
stream: { streamId: 's', cursor: '1' },
118122
payload: {
119123
op: 'upsert',
120-
resource: { type: 'table', id, title: 'Invoices', ...(viewId ? { viewId } : {}) },
124+
resource: {
125+
type: 'table',
126+
id,
127+
title: 'Invoices',
128+
...(viewId ? { viewId } : {}),
129+
...(clearViewId ? { clearViewId } : {}),
130+
},
121131
},
122132
} as PersistedStreamEventEnvelope
123133
}
@@ -188,4 +198,33 @@ describe('handleResourceEvent saved-view pins', () => {
188198
expect(deps.setResources).not.toHaveBeenCalled()
189199
expect(useTableViewPinStore.getState().pins['tbl-1']).toBeUndefined()
190200
})
201+
202+
it('clears the stored and pending pin when the agent deletes a saved view', () => {
203+
const open: MothershipResource = {
204+
type: 'table',
205+
id: 'tbl-1',
206+
title: 'Invoices',
207+
viewId: 'view-1',
208+
}
209+
useTableViewPinStore.getState().pin('tbl-1', 'view-1')
210+
const deps = makeStreamLoopDeps({
211+
addResource: vi.fn(() => false),
212+
resourcesRef: { current: [open] },
213+
})
214+
const ctx = { deps } as StreamLoopContext
215+
216+
handleResourceEvent(ctx, tableUpsertEvent('tbl-1', undefined, true))
217+
218+
expect(deps.addResource).toHaveBeenCalledWith({
219+
type: 'table',
220+
id: 'tbl-1',
221+
title: 'Invoices',
222+
clearViewId: true,
223+
})
224+
const updater = (deps.setResources as ReturnType<typeof vi.fn>).mock.calls[0][0] as (
225+
current: MothershipResource[]
226+
) => MothershipResource[]
227+
expect(updater([open])).toEqual([{ type: 'table', id: 'tbl-1', title: 'Invoices' }])
228+
expect(useTableViewPinStore.getState().pins['tbl-1']).toBeUndefined()
229+
})
191230
})

apps/sim/app/workspace/[workspaceId]/home/hooks/stream/handle-resource-event.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,12 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
4545
} = ctx.deps
4646
const onResourceEvent = onResourceEventRef.current
4747
const payload = parsed.payload
48+
const shouldClearViewId =
49+
payload.resource.type === 'table' && payload.resource.clearViewId === true
4850
// A saved view the agent just created or edited: the table opens on it, and
4951
// an already-open table switches to it.
5052
const pinnedViewId =
53+
!shouldClearViewId &&
5154
payload.resource.type === 'table' &&
5255
typeof payload.resource.viewId === 'string' &&
5356
payload.resource.viewId.trim()
@@ -60,6 +63,7 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
6063
typeof payload.resource.title === 'string' ? payload.resource.title : payload.resource.id,
6164
...(pinnedViewId ? { viewId: pinnedViewId } : {}),
6265
})
66+
const resourceUpdate = shouldClearViewId ? { ...resource, clearViewId: true as const } : resource
6367

6468
if (payload.op === MothershipStreamV1ResourceOp.remove) {
6569
const resourceType = resource.type
@@ -109,7 +113,7 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
109113
!shouldAutoActivatePreviewSession(previewForResource)))
110114
const wasAdded = shouldSuppressFileResourceActivation
111115
? !resourcesRef.current.some((r) => r.type === resource.type && r.id === resource.id)
112-
: addResource(resource)
116+
: addResource(resourceUpdate)
113117
if (shouldSuppressFileResourceActivation && wasAdded) {
114118
setResources((current) =>
115119
current.some((r) => r.type === resource.type && r.id === resource.id)
@@ -136,6 +140,17 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
136140
// Consumed by the embedded table once its views list carries the view —
137141
// which may be after the refetch below lands, or after the tab first opens.
138142
useTableViewPinStore.getState().pin(resource.id, pinnedViewId)
143+
} else if (shouldClearViewId) {
144+
setResources((current) =>
145+
current.some((r) => r.type === 'table' && r.id === resource.id && r.viewId !== undefined)
146+
? current.map((r) => {
147+
if (r.type !== 'table' || r.id !== resource.id) return r
148+
const { viewId: _viewId, ...unpinned } = r
149+
return unpinned
150+
})
151+
: current
152+
)
153+
useTableViewPinStore.getState().clear(resource.id)
139154
}
140155
invalidateResourceQueries(queryClient, workspaceId, resource.type, resource.id)
141156

apps/sim/app/workspace/[workspaceId]/home/hooks/stream/stream-context.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { RevealedSimKeysByMessage } from '@/lib/copilot/chat/sim-key-redact
55
import { captureRevealedSimKeys } from '@/lib/copilot/chat/sim-key-redaction'
66
import type { SyntheticFilePreviewPayload } from '@/lib/copilot/request/session'
77
import type { FilePreviewSession } from '@/lib/copilot/request/session/file-preview-session-contract'
8+
import type { MothershipResourceUpdate } from '@/lib/copilot/resources/types'
89
import {
910
createTurnModel,
1011
type TurnModel,
@@ -95,7 +96,7 @@ export interface StreamLoopDeps {
9596
setResources: Dispatch<SetStateAction<MothershipResource[]>>
9697
setActiveResourceId: Dispatch<SetStateAction<string | null>>
9798

98-
addResource: (resource: MothershipResource) => boolean
99+
addResource: (resource: MothershipResourceUpdate) => boolean
99100
removeResource: (resourceType: MothershipResourceType, resourceId: string) => void
100101
startClientWorkflowTool: (id: string, name: string, args: Record<string, unknown>) => void
101102
startClientLocalFilesystemTool: (id: string, name: string, args: Record<string, unknown>) => void

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

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,8 @@ import {
7070
BROWSER_SESSION_RESOURCE_ID,
7171
isAddressableResource,
7272
isEphemeralResource,
73+
type MothershipResourceUpdate,
74+
mergeChatResource,
7375
sanitizeChatResources,
7476
TERMINAL_SESSION_RESOURCE_ID,
7577
} from '@/lib/copilot/resources/types'
@@ -211,7 +213,7 @@ export interface UseChatReturn {
211213
resources: MothershipResource[]
212214
activeResourceId: string | null
213215
setActiveResourceId: (id: string | null) => void
214-
addResource: (resource: MothershipResource) => boolean
216+
addResource: (resource: MothershipResourceUpdate) => boolean
215217
removeResource: (resourceType: MothershipResourceType, resourceId: string) => void
216218
reorderResources: (resources: MothershipResource[]) => void
217219
messageQueue: QueuedMessage[]
@@ -1383,6 +1385,7 @@ export function useChat(
13831385
*/
13841386
const undisplayableResourcesRef = useRef<MothershipResource[]>([])
13851387
const pendingPersistResourceKeysRef = useRef<Set<string>>(new Set())
1388+
const pendingClearViewIdKeysRef = useRef<Set<string>>(new Set())
13861389
const inFlightResourceAddsRef = useRef<Map<string, Promise<unknown>>>(new Map())
13871390
const reorderNeededAfterFlushRef = useRef(false)
13881391

@@ -1642,6 +1645,7 @@ export function useChat(
16421645
useTableViewPinStore.getState().reset()
16431646
undisplayableResourcesRef.current = []
16441647
pendingPersistResourceKeysRef.current.clear()
1648+
pendingClearViewIdKeysRef.current.clear()
16451649
inFlightResourceAddsRef.current.clear()
16461650
reorderNeededAfterFlushRef.current = false
16471651
resetEphemeralPreviewState()
@@ -1679,9 +1683,18 @@ export function useChat(
16791683
const key = `${resource.type}:${resource.id}`
16801684
if (!pendingKeys.has(key)) continue
16811685
pendingKeys.delete(key)
1686+
const shouldClearViewId = pendingClearViewIdKeysRef.current.has(key)
16821687
const promise = requestJson(addMothershipChatResourceContract, {
1683-
body: { chatId, resource },
1688+
body: {
1689+
chatId,
1690+
resource,
1691+
...(shouldClearViewId ? { clearViewId: true as const } : {}),
1692+
},
16841693
})
1694+
.then((result) => {
1695+
if (shouldClearViewId) pendingClearViewIdKeysRef.current.delete(key)
1696+
return result
1697+
})
16851698
.catch((err) => {
16861699
pendingPersistResourceKeysRef.current.add(key)
16871700
logger.warn('Failed to flush pending resource; will retry on next hydration', err)
@@ -1798,20 +1811,30 @@ export function useChat(
17981811
const source = chatHistory?.messages.map(toDisplayMessage) ?? pendingMessages
17991812
return source.map((m) => restoreRevealedSimKeysForMessage(m, revealedSimKeysRef.current))
18001813
}, [chatHistory, pendingMessages])
1801-
const addResource = useCallback((resource: MothershipResource): boolean => {
1814+
const addResource = useCallback((resourceUpdate: MothershipResourceUpdate): boolean => {
18021815
// The single fan-in for tab creation, so the invariant lives here.
1803-
if (!isAddressableResource(resource)) {
1804-
logger.warn('Ignored a resource with no id', { type: resource.type, title: resource.title })
1816+
if (!isAddressableResource(resourceUpdate)) {
1817+
logger.warn('Ignored a resource with no id', {
1818+
type: resourceUpdate.type,
1819+
title: resourceUpdate.title,
1820+
})
18051821
return false
18061822
}
1807-
if (resourcesRef.current.some((r) => r.type === resource.type && r.id === resource.id)) {
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) {
18081828
return false
18091829
}
18101830

18111831
setResources((prev) => {
1812-
const exists = prev.some((r) => r.type === resource.type && r.id === resource.id)
1813-
if (exists) return prev
1814-
return [...prev, resource]
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))
18151838
})
18161839
// Synthetic result/preview panels are in-memory only. The browser tab
18171840
// metadata is persisted even though its live page remains desktop-owned.
@@ -1821,19 +1844,34 @@ export function useChat(
18211844

18221845
const persistChatId = chatIdRef.current ?? selectedChatIdRef.current
18231846
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+
}
18241853
// `resourcesRef` is written during render, so adds of the same resource in
18251854
// one tick all read the pre-render list and all pass the check above. State
18261855
// converges (the updater is idempotent) but each fired its own POST — 5-6
18271856
// per resource in production.
18281857
const alreadyPersisting =
18291858
inFlightResourceAddsRef.current.has(key) || pendingPersistResourceKeysRef.current.has(key)
18301859
if (alreadyPersisting) {
1831-
return true
1860+
pendingPersistResourceKeysRef.current.add(key)
1861+
return existing === undefined
18321862
}
18331863
if (persistChatId) {
18341864
const promise = requestJson(addMothershipChatResourceContract, {
1835-
body: { chatId: persistChatId, resource },
1865+
body: {
1866+
chatId: persistChatId,
1867+
resource,
1868+
...(shouldClearViewId ? { clearViewId: true as const } : {}),
1869+
},
18361870
})
1871+
.then((result) => {
1872+
if (shouldClearViewId) pendingClearViewIdKeysRef.current.delete(key)
1873+
return result
1874+
})
18371875
.catch((err) => {
18381876
pendingPersistResourceKeysRef.current.add(key)
18391877
logger.warn('Failed to persist resource; will retry on next hydration', err)
@@ -1845,7 +1883,7 @@ export function useChat(
18451883
} else {
18461884
pendingPersistResourceKeysRef.current.add(key)
18471885
}
1848-
return true
1886+
return existing === undefined
18491887
}, [])
18501888

18511889
const removeResource = useCallback((resourceType: MothershipResourceType, resourceId: string) => {
@@ -1856,6 +1894,7 @@ export function useChat(
18561894
if (isEphemeralResource({ type: resourceType, id: resourceId, title: '' })) return
18571895

18581896
const key = `${resourceType}:${resourceId}`
1897+
pendingClearViewIdKeysRef.current.delete(key)
18591898
const wasPending = pendingPersistResourceKeysRef.current.delete(key)
18601899
const inFlightAdd = inFlightResourceAddsRef.current.get(key)
18611900
if (wasPending && !inFlightAdd) return
@@ -2344,6 +2383,7 @@ export function useChat(
23442383
setActiveResourceId(null)
23452384
useTableViewPinStore.getState().reset()
23462385
pendingPersistResourceKeysRef.current.clear()
2386+
pendingClearViewIdKeysRef.current.clear()
23472387
inFlightResourceAddsRef.current.clear()
23482388
reorderNeededAfterFlushRef.current = false
23492389
resetEphemeralPreviewState()

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

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,29 @@ const copilotChatResourceItemSchema = z.object({
100100
viewId: z.string().min(1).optional(),
101101
})
102102

103-
export const addCopilotChatResourceBodySchema = z.object({
104-
chatId: z.string(),
105-
resource: copilotChatResourceItemSchema,
106-
})
103+
export const addCopilotChatResourceBodySchema = z
104+
.object({
105+
chatId: z.string(),
106+
resource: copilotChatResourceItemSchema,
107+
clearViewId: z.literal(true).optional(),
108+
})
109+
.superRefine((body, ctx) => {
110+
if (body.clearViewId !== true) return
111+
if (body.resource.type !== 'table') {
112+
ctx.addIssue({
113+
code: 'custom',
114+
path: ['clearViewId'],
115+
message: 'clearViewId is only valid for table resources',
116+
})
117+
}
118+
if (body.resource.viewId !== undefined) {
119+
ctx.addIssue({
120+
code: 'custom',
121+
path: ['resource', 'viewId'],
122+
message: 'viewId must be omitted when clearViewId is true',
123+
})
124+
}
125+
})
107126
export type AddCopilotChatResourceBody = z.input<typeof addCopilotChatResourceBodySchema>
108127

109128
export const removeCopilotChatResourceBodySchema = z.object({

apps/sim/lib/copilot/generated/mothership-stream-v1-schema.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,9 @@ export const MOTHERSHIP_STREAM_V1_SCHEMA: JsonSchema = {
363363
MothershipStreamV1ResourceDescriptor: {
364364
additionalProperties: false,
365365
properties: {
366+
clearViewId: {
367+
type: 'boolean',
368+
},
366369
id: {
367370
type: 'string',
368371
},

apps/sim/lib/copilot/generated/mothership-stream-v1.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,7 @@ export interface MothershipStreamV1ResourceUpsertPayload {
279279
resource: MothershipStreamV1ResourceDescriptor
280280
}
281281
export interface MothershipStreamV1ResourceDescriptor {
282+
clearViewId?: boolean
282283
id: string
283284
title?: string
284285
type: string

apps/sim/lib/copilot/request/session/contract.test.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -255,4 +255,24 @@ describe('resource event view pins', () => {
255255

256256
expect(isContractStreamEventEnvelope(event)).toBe(false)
257257
})
258+
259+
it('accepts an explicit pin clear and rejects a non-boolean directive', () => {
260+
const event = {
261+
...BASE_ENVELOPE,
262+
type: 'resource' as const,
263+
payload: {
264+
op: 'upsert' as const,
265+
resource: { id: 'tbl-1', type: 'table', title: 'Invoices', clearViewId: true },
266+
},
267+
}
268+
269+
expect(isContractStreamEventEnvelope(event)).toBe(true)
270+
expect(parsePersistedStreamEventEnvelope(event).ok).toBe(true)
271+
expect(
272+
isContractStreamEventEnvelope({
273+
...event,
274+
payload: { ...event.payload, resource: { ...event.payload.resource, clearViewId: 'yes' } },
275+
})
276+
).toBe(false)
277+
})
258278
})

0 commit comments

Comments
 (0)