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

Author: j15z

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

@@ -135,8 +135,13 @@ export const PATCH = withRouteHandler(async (req: NextRequest) => {
     const existing = sanitizeChatResources(
       Array.isArray(chat.resources) ? (chat.resources as ChatResource[]) : []
     )
-    const canonicalOrder = sanitizeChatResources(newOrder)
-    const existingKeys = new Set(existing.map((r) => `${r.type}:${r.id}`))
+    // The client echoes the tabs it holds; anything it does not carry (a view
+    // pin, a path) is taken from the stored entry rather than dropped.
+    const existingByKey = new Map(existing.map((r) => [`${r.type}:${r.id}`, r]))
+    const canonicalOrder = sanitizeChatResources(newOrder).map((r) =>
+      mergeChatResource(existingByKey.get(`${r.type}:${r.id}`), r)
+    )
+    const existingKeys = new Set(existingByKey.keys())
     const newKeys = new Set(canonicalOrder.map((r) => `${r.type}:${r.id}`))
 
     if (existingKeys.size !== newKeys.size || ![...existingKeys].every((k) => newKeys.has(k))) {

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

@@ -141,7 +141,12 @@ describe('handleResourceEvent saved-view pins', () => {
       title: 'Invoices',
       viewId: 'view-1',
     })
-    expect(deps.setResources).not.toHaveBeenCalled()
+    // The pin merge always runs; on a list that lacks the table it is a no-op.
+    const updater = (deps.setResources as ReturnType<typeof vi.fn>).mock.calls[0][0] as (
+      current: MothershipResource[]
+    ) => MothershipResource[]
+    const others: MothershipResource[] = [{ type: 'file', id: 'file-1', title: 'notes.md' }]
+    expect(updater(others)).toBe(others)
     expect(useTableViewPinStore.getState().pins['tbl-1']?.viewId).toBe('view-1')
     expect(mocks.invalidateResourceQueries).toHaveBeenCalledWith(
       deps.queryClient,

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

@@ -122,16 +122,17 @@ export function handleResourceEvent(ctx: StreamLoopContext, parsed: ResourceEven
     previewActivationOwnerRef.current.delete(completedPreviewHandoff.sessionId)
   }
   if (pinnedViewId) {
-    if (!wasAdded) {
-      // The tab already exists: carry the newest pin so a remount adopts it.
-      setResources((current) =>
-        current.some((r) => r.type === 'table' && r.id === resource.id && r.viewId !== pinnedViewId)
-          ? current.map((r) =>
-              r.type === 'table' && r.id === resource.id ? { ...r, viewId: pinnedViewId } : r
-            )
-          : current
-      )
-    }
+    // Carry the newest pin on an existing tab so a remount adopts it. Not gated
+    // on `wasAdded`: two upserts in one render both read the stale ref and both
+    // report "added", while only the first updater actually inserted — the
+    // updater is idempotent, so it simply runs every time.
+    setResources((current) =>
+      current.some((r) => r.type === 'table' && r.id === resource.id && r.viewId !== pinnedViewId)
+        ? current.map((r) =>
+            r.type === 'table' && r.id === resource.id ? { ...r, viewId: pinnedViewId } : r
+          )
+        : current
+    )
     // Consumed by the embedded table once its views list carries the view —
     // which may be after the refetch below lands, or after the tab first opens.
     useTableViewPinStore.getState().pin(resource.id, pinnedViewId)

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

@@ -136,6 +136,7 @@ import type {
   QueuedSendHandoffSeed,
 } from '@/stores/mothership-queue/types'
 import type { ChatContext } from '@/stores/panel'
+import { useTableViewPinStore } from '@/stores/table/view-pin/store'
 import { useTerminalConsoleStore } from '@/stores/terminal'
 import { useWorkflowRegistry } from '@/stores/workflows/registry/store'
 import type { WorkflowMetadata } from '@/stores/workflows/registry/types'
@@ -1637,6 +1638,8 @@ export function useChat(
     setTransportIdle()
     setResources([])
     setActiveResourceId(null)
+    // Pending view pins belong to the chat whose stream issued them.
+    useTableViewPinStore.getState().reset()
     undisplayableResourcesRef.current = []
     pendingPersistResourceKeysRef.current.clear()
     inFlightResourceAddsRef.current.clear()
@@ -2339,6 +2342,7 @@ export function useChat(
     setTransportIdle()
     setResources([])
     setActiveResourceId(null)
+    useTableViewPinStore.getState().reset()
     pendingPersistResourceKeysRef.current.clear()
     inFlightResourceAddsRef.current.clear()
     reorderNeededAfterFlushRef.current = false

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

@@ -99,16 +99,19 @@ export type RenameCopilotChatBody = z.input<typeof renameCopilotChatBodySchema>
 
 const copilotResourceTypeSchema = z.enum(PERSISTED_RESOURCE_TYPES)
 
+const copilotChatResourceItemSchema = z.object({
+  type: copilotResourceTypeSchema,
+  // Matches the bound the chat-send path enforces.
+  id: requiredFieldSchema('resource.id cannot be empty'),
+  title: z.string(),
+  // Saved view a table tab is pinned to (type "table" only). One schema for
+  // add and reorder, so a reorder round-trip can never strip the pin.
+  viewId: z.string().min(1).optional(),
+})
+
 export const addCopilotChatResourceBodySchema = z.object({
   chatId: z.string(),
-  resource: z.object({
-    type: copilotResourceTypeSchema,
-    // Matches the bound the chat-send path enforces.
-    id: requiredFieldSchema('resource.id cannot be empty'),
-    title: z.string(),
-    // Saved view a table tab is pinned to (type "table" only).
-    viewId: z.string().min(1).optional(),
-  }),
+  resource: copilotChatResourceItemSchema,
 })
 export type AddCopilotChatResourceBody = z.input<typeof addCopilotChatResourceBodySchema>
 
@@ -121,13 +124,7 @@ export type RemoveCopilotChatResourceBody = z.input<typeof removeCopilotChatReso
 
 export const reorderCopilotChatResourcesBodySchema = z.object({
   chatId: z.string(),
-  resources: z.array(
-    z.object({
-      type: copilotResourceTypeSchema,
-      id: z.string(),
-      title: z.string(),
-    })
-  ),
+  resources: z.array(copilotChatResourceItemSchema),
 })
 export type ReorderCopilotChatResourcesBody = z.input<typeof reorderCopilotChatResourcesBodySchema>
 

apps/sim/lib/copilot/resources/types.test.ts

@@ -183,3 +183,27 @@ describe('mergeChatResource', () => {
     expect(mergeChatResource(pinnedB, stored)).toBe(pinnedB)
   })
 })
+
+describe('mergeChatResource metadata', () => {
+  it('takes the metadata a newcomer defines and keeps what it omits', () => {
+    const placeholder = resource({ type: 'file', id: 'f1', title: 'File' })
+    const upgraded = mergeChatResource(placeholder, {
+      type: 'file',
+      id: 'f1',
+      title: 'notes.md',
+      path: 'files/notes.md',
+    })
+    expect(upgraded).toEqual({ type: 'file', id: 'f1', title: 'notes.md', path: 'files/notes.md' })
+
+    // A later re-add without a path keeps the stored one.
+    expect(mergeChatResource(upgraded, { type: 'file', id: 'f1', title: 'notes.md' })).toBe(
+      upgraded
+    )
+
+    const log = resource({ type: 'log', id: 'row-1', title: 'Run' })
+    expect(
+      mergeChatResource(log, { type: 'log', id: 'row-1', title: 'Run', executionId: 'exec-1' })
+        .executionId
+    ).toBe('exec-1')
+  })
+})

apps/sim/lib/copilot/resources/types.ts

@@ -214,23 +214,34 @@ export const GENERIC_RESOURCE_TITLES = new Set<string>([
 
 /**
  * Folds a re-added resource into the stored entry with the same type+id. The
- * stored title wins unless it was a placeholder. A table's saved-view pin is
- * replaced when the newcomer carries one — the tab reopens on the view the
- * agent touched last — and kept when it does not, so an unrelated row edit
- * never unpins the tab.
+ * stored title wins unless it was a placeholder. Every other field the
+ * newcomer defines replaces the stored one — a file's `path`, a log's
+ * `executionId`, a table's saved-view pin (the tab reopens on the view the
+ * agent touched last) — while a field the newcomer omits is kept, so an
+ * unrelated row edit never unpins a table. Returns `prev` itself when nothing
+ * changes, so callers can skip a no-op write.
  */
 export function mergeChatResource(
   prev: MothershipResource | undefined,
   next: MothershipResource
 ): MothershipResource {
   if (!prev) return next
-  const title =
-    GENERIC_RESOURCE_TITLES.has(prev.title) && !GENERIC_RESOURCE_TITLES.has(next.title)
-      ? next.title
-      : prev.title
-  const viewId = next.viewId ?? prev.viewId
-  if (title === prev.title && viewId === prev.viewId) return prev
-  return { ...prev, title, ...(viewId !== undefined ? { viewId } : {}) }
+  const merged: MothershipResource = {
+    ...prev,
+    ...(next.path !== undefined ? { path: next.path } : {}),
+    ...(next.viewId !== undefined ? { viewId: next.viewId } : {}),
+    ...(next.executionId !== undefined ? { executionId: next.executionId } : {}),
+    title:
+      GENERIC_RESOURCE_TITLES.has(prev.title) && !GENERIC_RESOURCE_TITLES.has(next.title)
+        ? next.title
+        : prev.title,
+  }
+  const unchanged =
+    merged.title === prev.title &&
+    merged.path === prev.path &&
+    merged.viewId === prev.viewId &&
+    merged.executionId === prev.executionId
+  return unchanged ? prev : merged
 }
 
 export const VFS_DIR_TO_RESOURCE: Record<string, MothershipResourceType> = {

apps/sim/lib/copilot/tools/server/table/create-table-view.test.ts

@@ -20,6 +20,7 @@ vi.mock('@/lib/copilot/application/execute-table-use-case', () => ({
 }))
 
 import { createTableViewServerTool } from '@/lib/copilot/tools/server/table/create-table-view'
+import { asOrchestrationError } from '@/lib/core/orchestration/types'
 import { createTableViewUseCase, listTableViewsUseCase } from '@/lib/table/application/views'
 
 const context = { userId: 'user-1', workspaceId: 'ws-1', copilotToolExecution: true } as never
@@ -103,7 +104,7 @@ describe('create_table_view', () => {
     })
   })
 
-  it('numbers an unnamed view after the ones the table already has and passes isDefault through', async () => {
+  it('leaves an omitted name to the service (numbered under the lock) and passes isDefault through', async () => {
     executeUseCase
       .mockResolvedValueOnce({ table, views: [{ id: 'view-0' }, { id: 'view-1' }] })
       .mockResolvedValueOnce({
@@ -112,22 +113,42 @@ describe('create_table_view', () => {
       })
 
     const result = await createTableViewServerTool.execute(
-      { tableId: 'tbl-1', isDefault: true },
+      { tableId: 'tbl-1', name: '   ', isDefault: true },
       context
     )
 
     expect(executeUseCase).toHaveBeenNthCalledWith(
       2,
       context,
       createTableViewUseCase,
-      { tableId: 'tbl-1', workspaceId: 'ws-1', name: 'View 3', config: {}, isDefault: true },
+      { tableId: 'tbl-1', workspaceId: 'ws-1', name: undefined, config: {}, isDefault: true },
       { tableId: 'tbl-1' }
     )
     expect(result.success).toBe(true)
+    expect(result.message).toContain('"View 3"')
     expect(result.message).toContain('as its default')
     expect(result.data?.view.isDefault).toBe(true)
   })
 
+  it("classifies an unknown column as the caller's mistake, before any write", async () => {
+    executeUseCase.mockResolvedValueOnce({ table, views: [] })
+
+    const failure = await createTableViewServerTool
+      .execute(
+        {
+          tableId: 'tbl-1',
+          name: 'Urgent',
+          config: { filter: { all: [{ field: 'priority', op: 'eq', value: 'high' }] } },
+        },
+        context
+      )
+      .catch((error: unknown) => error)
+
+    expect(asOrchestrationError(failure)?.code).toBe('validation')
+    expect(asOrchestrationError(failure)?.message).toMatch(/Unknown column\(s\): priority/)
+    expect(executeUseCase).toHaveBeenCalledTimes(1)
+  })
+
   it('refuses without a table id and without workspace context', async () => {
     expect(await createTableViewServerTool.execute({ tableId: '  ' }, context)).toEqual({
       success: false,

apps/sim/lib/copilot/tools/server/table/create-table-view.ts

@@ -20,8 +20,9 @@ interface CreateTableViewArgs {
 /**
  * The main agent's direct path to a new saved view (the table subagent goes
  * through table_views). One list read supplies the columns for name→id
- * translation and the count behind the default name; the create then lands in
- * a single transaction, default flag included. The result names the table so
+ * translation; the create then lands in a single locked transaction — default
+ * flag and, when no name was given, the `View N` fallback included, so two
+ * unnamed creates can never pick the same N. The result names the table so
  * resource extraction opens the panel pinned to the new view.
  */
 export const createTableViewServerTool: BaseServerTool<CreateTableViewArgs, TableViewToolResult> = {
@@ -39,7 +40,7 @@ export const createTableViewServerTool: BaseServerTool<CreateTableViewArgs, Tabl
       { tableId }
     )
     const columns = (listed.table.schema as TableSchema).columns
-    const name = params.name?.trim() || `View ${listed.views.length + 1}`
+    const name = params.name?.trim() || undefined
     const created = await executeCopilotTableUseCase(
       context,
       createTableViewUseCase,