Skip to content

Commit 288f023

Browse files
fix(copilot): disambiguate VFS upload paths to prevent stale-row reads
Same-name chat attachments (e.g. multiple "image.png" screenshots) collided on the VFS path uploads/<originalName>. The agent's read("uploads/image.png") returned whichever row Postgres ordered first — leading to the agent reading stale uploads instead of the latest one (real prod incident: trace 71f719d884194b6820851e2267b98cda). Adds a nullable display_name column on workspace_files plus a partial unique index on (chat_id, display_name) WHERE context='mothership' that spans the row's entire lifetime (NOT gated on deleted_at IS NULL). VFS paths handed to the LLM during a session must remain stable — soft-deleting a sibling cannot free a name slot the model has already been told about, since that would make read("uploads/<name>") silently resolve to a different file. Migration is a clean two-statement ADD COLUMN + CREATE UNIQUE INDEX with no backfill. Legacy rows keep display_name = NULL; readers coalesce to original_name. Pre-existing collisions in legacy data persist (the upload already happened — no fix possible without rewriting history) but new uploads get full disambiguation. NULLs are distinct in PG unique indexes, so legacy rows don't block index creation or new inserts. trackChatUpload allocates "image.png", then "image (2).png", "image (3).png", ... with retry on 23505. Returns the assigned displayName so the read-hint string given to the model uses it. Resolver matches displayName for new rows and falls back to original_name for legacy NULL rows, preserving the normalize-segment fallback for macOS U+202F screenshot names.
1 parent 6bc34c5 commit 288f023

11 files changed

Lines changed: 15778 additions & 45 deletions

File tree

apps/sim/lib/copilot/chat/payload.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ export async function buildCopilotRequestPayload(
238238
const filename = (f.filename ?? f.name ?? 'file') as string
239239
const mediaType = (f.media_type ?? f.mimeType ?? 'application/octet-stream') as string
240240
try {
241-
await trackChatUpload(
241+
const { displayName } = await trackChatUpload(
242242
params.workspaceId,
243243
userId,
244244
chatId,
@@ -248,13 +248,13 @@ export async function buildCopilotRequestPayload(
248248
f.size
249249
)
250250
const lines = [
251-
`File "${filename}" (${mediaType}, ${f.size} bytes) uploaded.`,
252-
`Read with: read("uploads/${filename}")`,
253-
`To save permanently: materialize_file(fileName: "${filename}")`,
251+
`File "${displayName}" (${mediaType}, ${f.size} bytes) uploaded.`,
252+
`Read with: read("uploads/${displayName}")`,
253+
`To save permanently: materialize_file(fileName: "${displayName}")`,
254254
]
255-
if (filename.endsWith('.json')) {
255+
if (displayName.endsWith('.json')) {
256256
lines.push(
257-
`To import as a workflow: materialize_file(fileName: "${filename}", operation: "import")`
257+
`To import as a workflow: materialize_file(fileName: "${displayName}", operation: "import")`
258258
)
259259
}
260260
uploadContexts.push({

apps/sim/lib/copilot/tools/handlers/materialize-file.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ function toFileRecord(row: typeof workspaceFiles.$inferSelect) {
2121
return {
2222
id: row.id,
2323
workspaceId: row.workspaceId || '',
24-
name: row.originalName,
24+
name: row.displayName ?? row.originalName,
2525
key: row.key,
2626
path: `${pathPrefix}${encodeURIComponent(row.key)}?context=mothership`,
2727
size: row.size,
@@ -45,7 +45,7 @@ async function executeSave(fileName: string, chatId: string): Promise<ToolCallRe
4545

4646
const [updated] = await db
4747
.update(workspaceFiles)
48-
.set({ context: 'workspace', chatId: null })
48+
.set({ context: 'workspace', chatId: null, originalName: row.displayName ?? row.originalName })
4949
.where(and(eq(workspaceFiles.id, row.id), isNull(workspaceFiles.deletedAt)))
5050
.returning({ id: workspaceFiles.id, originalName: workspaceFiles.originalName })
5151

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
5+
import { dbChainMock, dbChainMockFns, resetDbChainMock } from '@sim/testing'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
vi.mock('@sim/db', () => dbChainMock)
9+
10+
const { mockReadFileRecord } = vi.hoisted(() => ({
11+
mockReadFileRecord: vi.fn(),
12+
}))
13+
14+
vi.mock('@/lib/copilot/vfs/file-reader', () => ({
15+
readFileRecord: mockReadFileRecord,
16+
}))
17+
18+
import {
19+
findMothershipUploadRowByChatAndName,
20+
listChatUploads,
21+
readChatUpload,
22+
} from './upload-file-reader'
23+
24+
const CHAT_ID = '11111111-1111-1111-1111-111111111111'
25+
const NOW = new Date('2026-05-05T00:00:00.000Z')
26+
27+
function makeRow(overrides: Partial<Record<string, unknown>> = {}) {
28+
return {
29+
id: 'wf_1',
30+
key: 'mothership/abc/123-image.png',
31+
userId: 'user_1',
32+
workspaceId: 'ws_1',
33+
context: 'mothership',
34+
chatId: CHAT_ID,
35+
originalName: 'image.png',
36+
displayName: 'image.png',
37+
contentType: 'image/png',
38+
size: 1024,
39+
deletedAt: null,
40+
uploadedAt: NOW,
41+
updatedAt: NOW,
42+
...overrides,
43+
}
44+
}
45+
46+
/**
47+
* Resolver chain is `.where().orderBy(...).limit(1)`. The default chain mock makes
48+
* `orderBy` a terminal, so we wire a chainable `{limit}` for each call manually.
49+
*/
50+
function mockOrderByThenLimit(rows: unknown) {
51+
dbChainMockFns.orderBy.mockReturnValueOnce({ limit: dbChainMockFns.limit } as never)
52+
dbChainMockFns.limit.mockResolvedValueOnce(rows as never)
53+
}
54+
55+
describe('findMothershipUploadRowByChatAndName', () => {
56+
beforeEach(() => {
57+
vi.clearAllMocks()
58+
resetDbChainMock()
59+
})
60+
61+
it('matches by displayName for the first occurrence', async () => {
62+
const row = makeRow({ id: 'wf_1', displayName: 'image.png' })
63+
mockOrderByThenLimit([row])
64+
65+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, 'image.png')
66+
67+
expect(result).toEqual(row)
68+
})
69+
70+
it('matches by suffixed displayName for collision-disambiguated rows', async () => {
71+
const row = makeRow({ id: 'wf_2', displayName: 'image (2).png' })
72+
mockOrderByThenLimit([row])
73+
74+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, 'image (2).png')
75+
76+
expect(result?.id).toBe('wf_2')
77+
expect(result?.displayName).toBe('image (2).png')
78+
})
79+
80+
it('prefers the most recent row when legacy rows share the same originalName', async () => {
81+
// Pre-displayName legacy rows have displayName=null. Resolver's ORDER BY uploaded_at
82+
// DESC ensures the newest upload wins, fixing read("uploads/<name>") for legacy data.
83+
const newer = makeRow({
84+
id: 'wf_new',
85+
displayName: null,
86+
originalName: 'image.png',
87+
uploadedAt: new Date('2026-05-05T12:00:00.000Z'),
88+
})
89+
mockOrderByThenLimit([newer])
90+
91+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, 'image.png')
92+
93+
expect(result?.id).toBe('wf_new')
94+
})
95+
96+
it('returns null when no row matches and the fallback scan is empty', async () => {
97+
// First query: .where().orderBy().limit() returns [].
98+
mockOrderByThenLimit([])
99+
// Second query: .where().orderBy(...) (no .limit) — orderBy is the terminal.
100+
dbChainMockFns.orderBy.mockResolvedValueOnce([] as never)
101+
102+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, 'missing.png')
103+
104+
expect(result).toBeNull()
105+
})
106+
107+
it('falls back to normalized segment match when exact lookup misses (macOS U+202F)', async () => {
108+
// Model passes ASCII space; DB row was saved with U+202F (narrow no-break space).
109+
const macosName = 'Screenshot 2026-05-05 at 9.41.00 AM.png'
110+
const asciiName = 'Screenshot 2026-05-05 at 9.41.00 AM.png'
111+
const row = makeRow({ id: 'wf_3', displayName: macosName })
112+
113+
mockOrderByThenLimit([])
114+
dbChainMockFns.orderBy.mockResolvedValueOnce([row] as never)
115+
116+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, asciiName)
117+
118+
expect(result?.id).toBe('wf_3')
119+
})
120+
})
121+
122+
describe('listChatUploads', () => {
123+
beforeEach(() => {
124+
vi.clearAllMocks()
125+
resetDbChainMock()
126+
})
127+
128+
it('returns rows in upload order with name set to displayName', async () => {
129+
const rows = [
130+
makeRow({ id: 'a', displayName: 'image.png' }),
131+
makeRow({ id: 'b', displayName: 'image (2).png' }),
132+
makeRow({ id: 'c', displayName: 'image (3).png' }),
133+
]
134+
dbChainMockFns.orderBy.mockResolvedValueOnce(rows)
135+
136+
const result = await listChatUploads(CHAT_ID)
137+
138+
expect(result.map((r) => r.id)).toEqual(['a', 'b', 'c'])
139+
expect(result.map((r) => r.name)).toEqual(['image.png', 'image (2).png', 'image (3).png'])
140+
expect(result.every((r) => r.storageContext === 'mothership')).toBe(true)
141+
})
142+
143+
it('returns [] and does not throw when the DB query fails', async () => {
144+
dbChainMockFns.orderBy.mockRejectedValueOnce(new Error('boom'))
145+
const result = await listChatUploads(CHAT_ID)
146+
expect(result).toEqual([])
147+
})
148+
})
149+
150+
describe('readChatUpload', () => {
151+
beforeEach(() => {
152+
vi.clearAllMocks()
153+
resetDbChainMock()
154+
mockReadFileRecord.mockReset()
155+
})
156+
157+
it('reads the row resolved by the suffixed displayName', async () => {
158+
const row = makeRow({ id: 'wf_2', displayName: 'image (2).png' })
159+
mockOrderByThenLimit([row])
160+
mockReadFileRecord.mockResolvedValueOnce({ content: 'PNGDATA', totalLines: 1 })
161+
162+
const result = await readChatUpload('image (2).png', CHAT_ID)
163+
164+
expect(result).toEqual({ content: 'PNGDATA', totalLines: 1 })
165+
expect(mockReadFileRecord).toHaveBeenCalledWith(
166+
expect.objectContaining({ id: 'wf_2', name: 'image (2).png', storageContext: 'mothership' })
167+
)
168+
})
169+
170+
it('returns null when no row matches', async () => {
171+
mockOrderByThenLimit([])
172+
dbChainMockFns.orderBy.mockResolvedValueOnce([] as never)
173+
174+
const result = await readChatUpload('nope.png', CHAT_ID)
175+
176+
expect(result).toBeNull()
177+
expect(mockReadFileRecord).not.toHaveBeenCalled()
178+
})
179+
})

apps/sim/lib/copilot/tools/handlers/upload-file-reader.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,29 @@ import { db } from '@sim/db'
22
import { workspaceFiles } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { toError } from '@sim/utils/errors'
5-
import { and, eq, isNull } from 'drizzle-orm'
5+
import { and, asc, desc, eq, isNull, or } from 'drizzle-orm'
66
import { type FileReadResult, readFileRecord } from '@/lib/copilot/vfs/file-reader'
77
import { normalizeVfsSegment } from '@/lib/copilot/vfs/normalize-segment'
88
import { getServePathPrefix } from '@/lib/uploads'
99
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/workspace-file-manager'
1010

1111
const logger = createLogger('UploadFileReader')
1212

13+
/**
14+
* VFS-visible name. Pre-displayName-column rows have NULL displayName, so we coalesce
15+
* to originalName for them (those rows may still collide with each other — acceptable
16+
* since the upload already happened; only new rows get collision protection).
17+
*/
18+
function vfsName(row: typeof workspaceFiles.$inferSelect): string {
19+
return row.displayName ?? row.originalName
20+
}
21+
1322
function toWorkspaceFileRecord(row: typeof workspaceFiles.$inferSelect): WorkspaceFileRecord {
1423
const pathPrefix = getServePathPrefix()
1524
return {
1625
id: row.id,
1726
workspaceId: row.workspaceId || '',
18-
name: row.originalName,
27+
name: vfsName(row),
1928
key: row.key,
2029
path: `${pathPrefix}${encodeURIComponent(row.key)}?context=mothership`,
2130
size: row.size,
@@ -29,8 +38,14 @@ function toWorkspaceFileRecord(row: typeof workspaceFiles.$inferSelect): Workspa
2938
}
3039

3140
/**
32-
* Resolve a mothership upload row by `originalName`, preferring an exact DB match (limit 1) and
33-
* only scanning all chat uploads when that misses (e.g. macOS U+202F vs ASCII space in the name).
41+
* Resolve a mothership upload row by VFS name (the collision-disambiguated `displayName`
42+
* for new rows, or `originalName` for legacy rows that predate the column). Prefers an
43+
* exact DB match; falls back to a normalized scan when the model passes a visually
44+
* equivalent name (e.g. macOS U+202F vs ASCII space in screenshot filenames).
45+
*
46+
* On ambiguity (multiple legacy rows sharing the same originalName in one chat — the
47+
* pre-displayName collision case), returns the most recent upload. New rows are unique
48+
* by index so this only affects pre-fix data.
3449
*/
3550
export async function findMothershipUploadRowByChatAndName(
3651
chatId: string,
@@ -43,10 +58,14 @@ export async function findMothershipUploadRowByChatAndName(
4358
and(
4459
eq(workspaceFiles.chatId, chatId),
4560
eq(workspaceFiles.context, 'mothership'),
46-
eq(workspaceFiles.originalName, fileName),
61+
or(
62+
eq(workspaceFiles.displayName, fileName),
63+
and(isNull(workspaceFiles.displayName), eq(workspaceFiles.originalName, fileName))
64+
),
4765
isNull(workspaceFiles.deletedAt)
4866
)
4967
)
68+
.orderBy(desc(workspaceFiles.uploadedAt), desc(workspaceFiles.id))
5069
.limit(1)
5170

5271
if (exactRows[0]) {
@@ -63,13 +82,14 @@ export async function findMothershipUploadRowByChatAndName(
6382
isNull(workspaceFiles.deletedAt)
6483
)
6584
)
85+
.orderBy(desc(workspaceFiles.uploadedAt), desc(workspaceFiles.id))
6686

6787
const segmentKey = normalizeVfsSegment(fileName)
68-
return allRows.find((r) => normalizeVfsSegment(r.originalName) === segmentKey) ?? null
88+
return allRows.find((r) => normalizeVfsSegment(vfsName(r)) === segmentKey) ?? null
6989
}
7090

7191
/**
72-
* List all chat-scoped uploads for a given chat.
92+
* List all chat-scoped uploads for a given chat in upload order.
7393
*/
7494
export async function listChatUploads(chatId: string): Promise<WorkspaceFileRecord[]> {
7595
try {
@@ -83,6 +103,7 @@ export async function listChatUploads(chatId: string): Promise<WorkspaceFileReco
83103
isNull(workspaceFiles.deletedAt)
84104
)
85105
)
106+
.orderBy(asc(workspaceFiles.uploadedAt), asc(workspaceFiles.id))
86107

87108
return rows.map(toWorkspaceFileRecord)
88109
} catch (err) {

0 commit comments

Comments
 (0)