Skip to content

Commit 7395a37

Browse files
j15zwaleedlatif1claude
authored
feat(copilot): open the table on the view table_views just wrote (#7166)
* feat(copilot): add create_table_view and edit_table_view Direct main-agent tools for saved table views. create_table_view takes a table id (optional name, config, isDefault) and returns the view id; edit_table_view takes a view id plus a config patch and resolves the owning table from the view. Both results name the table and view, so the resource panel opens the table pinned to that view, and an already-open table switches to it once its views list carries the id (view-pin store). viewId now rides the resource stream descriptor and chat-resource persistence so the pin survives reopening the chat. * fix(copilot): address review findings on table view tools - edit_table_view resolves the view's table under a workspace-only context (no table scope exists yet for the delegated principal), then re-enters the table-scoped read and update with that id - updateTableView takes the per-table views lock when promoting, so it serializes with default-on-create instead of racing the unique index - the View N fallback is chosen inside the locked create - unknown column names are classified as validation errors in the shared translation, so the model sees which column it got wrong - pending view pins are reset when a chat is torn down or switched - add and reorder share one chat-resource item schema; reorder merges incoming entries with stored ones so pins and paths survive - mergeChatResource keeps every field the newcomer defines - the pin merge runs for every pinned upsert, not gated on wasAdded * refactor(copilot): drop the direct view tools, pin views through table_views Views stay with the table subagent's multiplexed table_views; the orchestrator delegates as before. Its create/update/set-default results now name the table and view they wrote, and resource extraction turns that into the pinned table resource, so the panel opens (or switches) the table on that view. Unknown column names are classified as validation errors, and create_view's isDefault lands in the same locked transaction as the insert. The stream/persistence plumbing for viewId, the pin store, and the lock on default promotion are unchanged. * chore(copilot): sync table view update semantics * fix(copilot): sync table view sort item schema * fix(copilot): persist table view pin updates * fix(tables): reconcile agent view pins * fix(copilot): type resource update directives * fix(tables): serialize default view demotions * fix(copilot): preserve view pin clear requests * fix(copilot): serialize resource view updates * fix(copilot): close resource persistence races * fix(copilot): retain resource removal intent * fix: isolate copilot resource persistence by chat * fix(tables): restore view when returning to chat * fix(copilot): bound resource-write locks and repair reorder persistence Review follow-ups on the saved-view pinning work. Correctness: - Reorder persistence was parked by ANY pending write. A repeatedly failing update to an already-stored resource (a view pin) blocked tab ordering for the rest of the session; gate on unpersisted writes only, which are the ones the server's identity check can actually reject. - A parked reorder body that the server rejects was re-parked verbatim, so a tab closed after the order was captured poisoned it permanently. Discard on 400; keep retrying everything transient. - adoptScope merged the provisional and chat-scoped updates in the wrong order, letting an older pending write overwrite a newer one. - A view pin that arrived before the table finished its first adoption was dropped for good when the table data resolved after the views list. The stream path self-rescued through query invalidation; the restore path did not. Re-run the effect when adoption becomes possible. - Reordering a chat holding a legacy duplicate row 400'd forever. Compare identity sets so the duplicate collapses on write instead. - mergeChatResource aliased the caller's object into React state, the query cache and the pending-write queue at once. Copy it. Robustness: - The new copilot_chats FOR UPDATE transactions had no lock_timeout, and neither the pool nor the deployment sets one. finalizeAssistantTurn holds that same row across an assistant-message append, so a waiter could park a pool connection indefinitely. Bound all five writers. - mergeChatResource's field list is now one declaration that fails to compile when MothershipResource gains a field, rather than silently dropping it from both the merge and its no-op check. - Extraction can no longer emit viewId and clearViewId together, a pair the wire contract rejects and the merge would resolve to neither. - Restrict the eager view-id URL write to embedded tables, leaving standalone table behaviour identical to staging. - Drop the queue's unreachable unscoped bucket, its uncalled clear(), and its test-only getPendingUpdates(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FwmaLmAXSK2hsPBGZmkPnT * fix(copilot): hold a reorder for pending deletes too The reorder gate landed one case short: a delete that has not reached the server leaves the server holding a resource the client's order omits, so the order fails its identity check exactly as an unlanded add does. Gating only on unpersisted adds let that order fire and be discarded as unsatisfiable, losing the tab order until the next reorder or hydration. Name the predicate for what it actually decides — whether a pending write changes WHICH resources the chat holds — and cover both directions. A failing update to an already-stored resource still does not park the order, which is what the gate was narrowed for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FwmaLmAXSK2hsPBGZmkPnT --------- Co-authored-by: Waleed Latif <walif6@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 39307bd commit 7395a37

40 files changed

Lines changed: 2575 additions & 415 deletions

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

Lines changed: 102 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,16 @@ import {
1616
createNotFoundResponse,
1717
createUnauthorizedResponse,
1818
} from '@/lib/copilot/request/http'
19-
import type { ChatResource } from '@/lib/copilot/resources/persistence'
19+
import {
20+
type ChatResource,
21+
serializeChatResourceWrite,
22+
setChatResourceTxTimeouts,
23+
} from '@/lib/copilot/resources/persistence'
24+
import type { MothershipResourceUpdate } from '@/lib/copilot/resources/types'
2025
import {
2126
canonicalizeDesktopSessionResource,
22-
GENERIC_RESOURCE_TITLES,
27+
mergeChatResource,
28+
reorderStoredChatResources,
2329
sanitizeChatResources,
2430
} from '@/lib/copilot/resources/types'
2531
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
@@ -43,59 +49,56 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
4349
}
4450
)
4551
if (!parsed.success) return parsed.response
46-
const { chatId, resource: requestedResource } = parsed.data.body
52+
const { chatId, resource: requestedResource, clearViewId } = parsed.data.body
4753
const resource = canonicalizeDesktopSessionResource(requestedResource)
54+
const resourceUpdate: MothershipResourceUpdate =
55+
clearViewId === true ? { ...resource, clearViewId: true } : resource
4856

4957
// Ephemeral UI tab (client does not POST this; guard for old clients / bugs).
5058
if (resource.id === 'streaming-file') {
5159
return NextResponse.json({ success: true })
5260
}
5361

54-
const [chat] = await db
55-
.select({ resources: copilotChats.resources })
56-
.from(copilotChats)
57-
.where(
58-
and(
62+
const merged = await serializeChatResourceWrite(chatId, () =>
63+
db.transaction(async (tx) => {
64+
await setChatResourceTxTimeouts(tx)
65+
const scope = and(
5966
eq(copilotChats.id, chatId),
6067
eq(copilotChats.userId, userId),
6168
isNull(copilotChats.deletedAt)
6269
)
63-
)
64-
.limit(1)
70+
const [chat] = await tx
71+
.select({ resources: copilotChats.resources })
72+
.from(copilotChats)
73+
.where(scope)
74+
.for('update')
75+
.limit(1)
6576

66-
if (!chat) {
67-
return createNotFoundResponse('Chat not found or unauthorized')
68-
}
77+
if (!chat) return null
6978

70-
const existing = sanitizeChatResources(
71-
Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : []
72-
)
73-
const key = `${resource.type}:${resource.id}`
74-
const prev = existing.find((r) => `${r.type}:${r.id}` === key)
75-
76-
let merged: ChatResource[]
77-
if (prev) {
78-
if (GENERIC_RESOURCE_TITLES.has(prev.title) && !GENERIC_RESOURCE_TITLES.has(resource.title)) {
79-
merged = existing.map((r) =>
80-
`${r.type}:${r.id}` === key ? { ...r, title: resource.title } : r
79+
const existing = sanitizeChatResources(
80+
Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : []
8181
)
82-
} else {
83-
merged = existing
84-
}
85-
} else {
86-
merged = [...existing, resource]
87-
}
82+
const key = `${resource.type}:${resource.id}`
83+
const prev = existing.find((r) => `${r.type}:${r.id}` === key)
84+
const next: ChatResource[] = prev
85+
? existing.map((r) =>
86+
`${r.type}:${r.id}` === key ? mergeChatResource(r, resourceUpdate) : r
87+
)
88+
: [...existing, mergeChatResource(undefined, resourceUpdate)]
89+
90+
await tx
91+
.update(copilotChats)
92+
.set({ resources: sql`${JSON.stringify(next)}::jsonb`, updatedAt: new Date() })
93+
.where(scope)
94+
95+
return next
96+
})
97+
)
8898

89-
await db
90-
.update(copilotChats)
91-
.set({ resources: sql`${JSON.stringify(merged)}::jsonb`, updatedAt: new Date() })
92-
.where(
93-
and(
94-
eq(copilotChats.id, chatId),
95-
eq(copilotChats.userId, userId),
96-
isNull(copilotChats.deletedAt)
97-
)
98-
)
99+
if (!merged) {
100+
return createNotFoundResponse('Chat not found or unauthorized')
101+
}
99102

100103
logger.info('Added resource to chat', { chatId, resource })
101104

@@ -125,44 +128,45 @@ export const PATCH = withRouteHandler(async (req: NextRequest) => {
125128
if (!parsed.success) return parsed.response
126129
const { chatId, resources: newOrder } = parsed.data.body
127130

128-
const [chat] = await db
129-
.select({ resources: copilotChats.resources })
130-
.from(copilotChats)
131-
.where(
132-
and(
131+
const canonicalOrder = await serializeChatResourceWrite(chatId, () =>
132+
db.transaction(async (tx): Promise<ChatResource[] | null | undefined> => {
133+
await setChatResourceTxTimeouts(tx)
134+
const scope = and(
133135
eq(copilotChats.id, chatId),
134136
eq(copilotChats.userId, userId),
135137
isNull(copilotChats.deletedAt)
136138
)
137-
)
138-
.limit(1)
139+
const [chat] = await tx
140+
.select({ resources: copilotChats.resources })
141+
.from(copilotChats)
142+
.where(scope)
143+
.for('update')
144+
.limit(1)
139145

140-
if (!chat) {
141-
return createNotFoundResponse('Chat not found or unauthorized')
142-
}
146+
if (!chat) return undefined
147+
148+
const existing = sanitizeChatResources(
149+
Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : []
150+
)
151+
const next = reorderStoredChatResources(existing, newOrder)
152+
if (!next) return null
153+
154+
await tx
155+
.update(copilotChats)
156+
.set({ resources: sql`${JSON.stringify(next)}::jsonb`, updatedAt: new Date() })
157+
.where(scope)
143158

144-
const existing = sanitizeChatResources(
145-
Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : []
159+
return next
160+
})
146161
)
147-
const canonicalOrder = sanitizeChatResources(newOrder)
148-
const existingKeys = new Set(existing.map((r) => `${r.type}:${r.id}`))
149-
const newKeys = new Set(canonicalOrder.map((r) => `${r.type}:${r.id}`))
150162

151-
if (existingKeys.size !== newKeys.size || ![...existingKeys].every((k) => newKeys.has(k))) {
163+
if (canonicalOrder === undefined) {
164+
return createNotFoundResponse('Chat not found or unauthorized')
165+
}
166+
if (!canonicalOrder) {
152167
return createBadRequestResponse('Reordered resources must match existing resources')
153168
}
154169

155-
await db
156-
.update(copilotChats)
157-
.set({ resources: sql`${JSON.stringify(canonicalOrder)}::jsonb`, updatedAt: new Date() })
158-
.where(
159-
and(
160-
eq(copilotChats.id, chatId),
161-
eq(copilotChats.userId, userId),
162-
isNull(copilotChats.deletedAt)
163-
)
164-
)
165-
166170
logger.info('Reordered resources for chat', { chatId, count: canonicalOrder.length })
167171

168172
return NextResponse.json({ success: true, resources: canonicalOrder })
@@ -191,39 +195,45 @@ export const DELETE = withRouteHandler(async (req: NextRequest) => {
191195
if (!parsed.success) return parsed.response
192196
const { chatId, resourceType, resourceId } = parsed.data.body
193197

194-
// Old builds could persist an inner browser/terminal tab id. Closing the
195-
// singleton panel removes every legacy row of that type so it cannot be
196-
// canonicalized back into view on the next hydration.
197-
const removePredicate =
198-
resourceType === 'browser' || resourceType === 'terminal'
199-
? sql`elem->>'type' = ${resourceType}`
200-
: sql`elem->>'type' = ${resourceType} AND elem->>'id' = ${resourceId}`
201-
202-
const [updated] = await db
203-
.update(copilotChats)
204-
.set({
205-
resources: sql`COALESCE((
206-
SELECT jsonb_agg(elem)
207-
FROM jsonb_array_elements(${copilotChats.resources}) elem
208-
WHERE NOT (${removePredicate})
209-
), '[]'::jsonb)`,
210-
updatedAt: new Date(),
211-
})
212-
.where(
213-
and(
198+
const merged = await serializeChatResourceWrite(chatId, () =>
199+
db.transaction(async (tx) => {
200+
await setChatResourceTxTimeouts(tx)
201+
const scope = and(
214202
eq(copilotChats.id, chatId),
215203
eq(copilotChats.userId, userId),
216204
isNull(copilotChats.deletedAt)
217205
)
218-
)
219-
.returning({ resources: copilotChats.resources })
206+
const [chat] = await tx
207+
.select({ resources: copilotChats.resources })
208+
.from(copilotChats)
209+
.where(scope)
210+
.for('update')
211+
.limit(1)
220212

221-
if (!updated) {
213+
if (!chat) return null
214+
215+
const existing = sanitizeChatResources(
216+
Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : []
217+
)
218+
const removeAllOfType = resourceType === 'browser' || resourceType === 'terminal'
219+
const next = existing.filter(
220+
(resource) =>
221+
resource.type !== resourceType || (!removeAllOfType && resource.id !== resourceId)
222+
)
223+
224+
await tx
225+
.update(copilotChats)
226+
.set({ resources: sql`${JSON.stringify(next)}::jsonb`, updatedAt: new Date() })
227+
.where(scope)
228+
229+
return next
230+
})
231+
)
232+
233+
if (!merged) {
222234
return createNotFoundResponse('Chat not found or unauthorized')
223235
}
224236

225-
const merged = Array.isArray(updated.resources) ? (updated.resources as ChatResource[]) : []
226-
227237
logger.info('Removed resource from chat', { chatId, resourceType, resourceId })
228238

229239
return NextResponse.json({ success: true, resources: merged })

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.test.tsx‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act, type ReactNode } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
vi.mock('@/app/workspace/[workspaceId]/tables/[tableId]/table', () => ({
9+
Table: () => null,
10+
}))
11+
vi.mock(
12+
'@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-session',
13+
() => ({ BrowserSession: () => null })
14+
)
15+
vi.mock(
16+
'@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/terminal-session/terminal-session',
17+
() => ({ TerminalSession: () => null })
18+
)
19+
20+
import { ResourceContent } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content'
21+
import type { MothershipResource } from '@/app/workspace/[workspaceId]/home/types'
22+
import { useTableViewPinStore } from '@/stores/table/view-pin/store'
23+
24+
describe('ResourceContent table view handoff', () => {
25+
let container: HTMLDivElement
26+
let root: Root
27+
28+
beforeEach(() => {
29+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
30+
useTableViewPinStore.getState().reset()
31+
container = document.createElement('div')
32+
root = createRoot(container)
33+
})
34+
35+
afterEach(() => {
36+
act(() => root.unmount())
37+
useTableViewPinStore.getState().reset()
38+
})
39+
40+
function render(resource: MothershipResource) {
41+
act(() => {
42+
root.render(
43+
(
44+
<ResourceContent
45+
workspaceId='workspace-1'
46+
desktopScopeId='chat:chat-1'
47+
resource={resource}
48+
/>
49+
) as ReactNode
50+
)
51+
})
52+
}
53+
54+
it('hands off a saved view that arrives after the embedded table mounts', () => {
55+
const table: MothershipResource = {
56+
type: 'table',
57+
id: 'table-1',
58+
title: 'Invoices',
59+
}
60+
render(table)
61+
expect(useTableViewPinStore.getState().pins['table-1']).toBeUndefined()
62+
63+
render({ ...table, viewId: 'view-edited' })
64+
const pin = useTableViewPinStore.getState().pins['table-1']
65+
expect(pin?.viewId).toBe('view-edited')
66+
67+
render({ ...table, viewId: 'view-edited' })
68+
expect(useTableViewPinStore.getState().pins['table-1']?.seq).toBe(pin?.seq)
69+
})
70+
})

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/resource-content.tsx‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import { useWorkflows } from '@/hooks/queries/workflows'
6262
import { useWorkspaceFiles } from '@/hooks/queries/workspace-files'
6363
import { useSettingsNavigation } from '@/hooks/use-settings-navigation'
6464
import { useExecutionStore } from '@/stores/execution/store'
65+
import { useTableViewPinStore } from '@/stores/table/view-pin/store'
6566
import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
6667

6768
const Workflow = lazy(() => import('@/app/workspace/[workspaceId]/w/[workflowId]/workflow'))
@@ -178,6 +179,25 @@ export const ResourceContent = memo(function ResourceContent({
178179
visible = true,
179180
onBrowserOverlayControllerChange,
180181
}: ResourceContentProps) {
182+
const observedTableViewRef = useRef(
183+
resource.type === 'table' ? { tableId: resource.id, viewId: resource.viewId } : null
184+
)
185+
186+
useEffect(() => {
187+
const previous = observedTableViewRef.current
188+
const next =
189+
resource.type === 'table' ? { tableId: resource.id, viewId: resource.viewId } : null
190+
observedTableViewRef.current = next
191+
if (!next?.viewId || (previous?.tableId === next.tableId && previous.viewId === next.viewId)) {
192+
return
193+
}
194+
/**
195+
* `initialViewId` owns the first table adoption. If refreshed chat data
196+
* supplies it later, use the same one-shot handoff as live stream events.
197+
*/
198+
useTableViewPinStore.getState().pin(next.tableId, next.viewId)
199+
}, [resource.id, resource.type, resource.viewId])
200+
181201
const streamFileName = previewSession?.fileName || 'file.md'
182202
const syntheticFile = useMemo(() => {
183203
const ext = getFileExtension(streamFileName)

‎apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,9 @@ const RESOURCE_INVALIDATORS: Record<
300300
table: (qc, _wId, id) => {
301301
qc.invalidateQueries({ queryKey: tableKeys.lists() })
302302
qc.invalidateQueries({ queryKey: tableKeys.detail(id) })
303+
// A view the agent just created must be in the list before the embedded
304+
// table can switch to it; see the view-pin store.
305+
qc.invalidateQueries({ queryKey: tableKeys.views(id) })
303306
},
304307
file: (qc, wId, id) => {
305308
qc.invalidateQueries({ queryKey: workspaceFilesKeys.lists() })

0 commit comments

Comments
 (0)