Skip to content

Commit 99128a3

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>, causing read("uploads/image.png") to return whichever row Postgres ordered first — leading to the agent reading stale uploads instead of the latest one. Adds displayName column + partial unique index on (chat_id, display_name) WHERE context='mothership'. trackChatUpload allocates "image.png", then "image (2).png", "image (3).png", ... with retry on 23505. The read-hint string returned to the model uses displayName so subsequent read/materialize calls resolve unambiguously. Resolver now matches on displayName with the existing normalizeVfsSegment fallback for macOS U+202F screenshot names.
1 parent 31cfb74 commit 99128a3

11 files changed

Lines changed: 15735 additions & 44 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,
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 })
4949
.where(and(eq(workspaceFiles.id, row.id), isNull(workspaceFiles.deletedAt)))
5050
.returning({ id: workspaceFiles.id, originalName: workspaceFiles.originalName })
5151

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
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+
describe('findMothershipUploadRowByChatAndName', () => {
47+
beforeEach(() => {
48+
vi.clearAllMocks()
49+
resetDbChainMock()
50+
})
51+
52+
it('matches by displayName for the first occurrence', async () => {
53+
const row = makeRow({ id: 'wf_1', displayName: 'image.png' })
54+
dbChainMockFns.limit.mockResolvedValueOnce([row])
55+
56+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, 'image.png')
57+
58+
expect(result).toEqual(row)
59+
})
60+
61+
it('matches by suffixed displayName for collision-disambiguated rows', async () => {
62+
const row = makeRow({ id: 'wf_2', displayName: 'image (2).png' })
63+
dbChainMockFns.limit.mockResolvedValueOnce([row])
64+
65+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, 'image (2).png')
66+
67+
expect(result?.id).toBe('wf_2')
68+
expect(result?.displayName).toBe('image (2).png')
69+
})
70+
71+
it('returns null when no row matches and the fallback scan is empty', async () => {
72+
// First query: .where().limit(1) — empty.
73+
dbChainMockFns.limit.mockResolvedValueOnce([])
74+
// Second query awaits .where(...) directly. Default `where` returns the chainable
75+
// (consumed by call #1); call #2 must return a thenable so `await` resolves to [].
76+
dbChainMockFns.where.mockImplementationOnce(() => ({ limit: dbChainMockFns.limit }) as never)
77+
dbChainMockFns.where.mockImplementationOnce(() => Promise.resolve([]) as never)
78+
79+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, 'missing.png')
80+
81+
expect(result).toBeNull()
82+
})
83+
84+
it('falls back to normalized segment match when exact lookup misses (macOS U+202F)', async () => {
85+
// Model passes ASCII space; DB row was saved with U+202F (narrow no-break space).
86+
const macosName = 'Screenshot 2026-05-05 at 9.41.00 AM.png'
87+
const asciiName = 'Screenshot 2026-05-05 at 9.41.00 AM.png'
88+
const row = makeRow({ id: 'wf_3', displayName: macosName })
89+
90+
dbChainMockFns.limit.mockResolvedValueOnce([])
91+
dbChainMockFns.where.mockImplementationOnce(() => ({ limit: dbChainMockFns.limit }) as never)
92+
dbChainMockFns.where.mockImplementationOnce(() => Promise.resolve([row]) as never)
93+
94+
const result = await findMothershipUploadRowByChatAndName(CHAT_ID, asciiName)
95+
96+
expect(result?.id).toBe('wf_3')
97+
})
98+
})
99+
100+
describe('listChatUploads', () => {
101+
beforeEach(() => {
102+
vi.clearAllMocks()
103+
resetDbChainMock()
104+
})
105+
106+
it('returns rows in upload order with name set to displayName', async () => {
107+
const rows = [
108+
makeRow({ id: 'a', displayName: 'image.png' }),
109+
makeRow({ id: 'b', displayName: 'image (2).png' }),
110+
makeRow({ id: 'c', displayName: 'image (3).png' }),
111+
]
112+
dbChainMockFns.orderBy.mockResolvedValueOnce(rows)
113+
114+
const result = await listChatUploads(CHAT_ID)
115+
116+
expect(result.map((r) => r.id)).toEqual(['a', 'b', 'c'])
117+
expect(result.map((r) => r.name)).toEqual(['image.png', 'image (2).png', 'image (3).png'])
118+
expect(result.every((r) => r.storageContext === 'mothership')).toBe(true)
119+
})
120+
121+
it('returns [] and does not throw when the DB query fails', async () => {
122+
dbChainMockFns.orderBy.mockRejectedValueOnce(new Error('boom'))
123+
const result = await listChatUploads(CHAT_ID)
124+
expect(result).toEqual([])
125+
})
126+
})
127+
128+
describe('readChatUpload', () => {
129+
beforeEach(() => {
130+
vi.clearAllMocks()
131+
resetDbChainMock()
132+
mockReadFileRecord.mockReset()
133+
})
134+
135+
it('reads the row resolved by the suffixed displayName', async () => {
136+
const row = makeRow({ id: 'wf_2', displayName: 'image (2).png' })
137+
dbChainMockFns.limit.mockResolvedValueOnce([row])
138+
mockReadFileRecord.mockResolvedValueOnce({ content: 'PNGDATA', totalLines: 1 })
139+
140+
const result = await readChatUpload('image (2).png', CHAT_ID)
141+
142+
expect(result).toEqual({ content: 'PNGDATA', totalLines: 1 })
143+
expect(mockReadFileRecord).toHaveBeenCalledWith(
144+
expect.objectContaining({ id: 'wf_2', name: 'image (2).png', storageContext: 'mothership' })
145+
)
146+
})
147+
148+
it('returns null when no row matches', async () => {
149+
dbChainMockFns.limit.mockResolvedValueOnce([])
150+
dbChainMockFns.where.mockResolvedValueOnce([])
151+
152+
const result = await readChatUpload('nope.png', CHAT_ID)
153+
154+
expect(result).toBeNull()
155+
expect(mockReadFileRecord).not.toHaveBeenCalled()
156+
})
157+
})

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ 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, eq, isNull } 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'
@@ -15,7 +15,7 @@ function toWorkspaceFileRecord(row: typeof workspaceFiles.$inferSelect): Workspa
1515
return {
1616
id: row.id,
1717
workspaceId: row.workspaceId || '',
18-
name: row.originalName,
18+
name: row.displayName,
1919
key: row.key,
2020
path: `${pathPrefix}${encodeURIComponent(row.key)}?context=mothership`,
2121
size: row.size,
@@ -29,8 +29,10 @@ function toWorkspaceFileRecord(row: typeof workspaceFiles.$inferSelect): Workspa
2929
}
3030

3131
/**
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).
32+
* Resolve a mothership upload row by `displayName` (the collision-disambiguated name
33+
* exposed via `uploads/<displayName>` in the VFS). Prefers an exact DB match; falls back
34+
* to a normalized scan when the model passes a visually-equivalent name (e.g. macOS U+202F
35+
* narrow no-break space vs ASCII space in screenshot filenames).
3436
*/
3537
export async function findMothershipUploadRowByChatAndName(
3638
chatId: string,
@@ -43,7 +45,7 @@ export async function findMothershipUploadRowByChatAndName(
4345
and(
4446
eq(workspaceFiles.chatId, chatId),
4547
eq(workspaceFiles.context, 'mothership'),
46-
eq(workspaceFiles.originalName, fileName),
48+
eq(workspaceFiles.displayName, fileName),
4749
isNull(workspaceFiles.deletedAt)
4850
)
4951
)
@@ -65,11 +67,11 @@ export async function findMothershipUploadRowByChatAndName(
6567
)
6668

6769
const segmentKey = normalizeVfsSegment(fileName)
68-
return allRows.find((r) => normalizeVfsSegment(r.originalName) === segmentKey) ?? null
70+
return allRows.find((r) => normalizeVfsSegment(r.displayName) === segmentKey) ?? null
6971
}
7072

7173
/**
72-
* List all chat-scoped uploads for a given chat.
74+
* List all chat-scoped uploads for a given chat in upload order.
7375
*/
7476
export async function listChatUploads(chatId: string): Promise<WorkspaceFileRecord[]> {
7577
try {
@@ -83,6 +85,7 @@ export async function listChatUploads(chatId: string): Promise<WorkspaceFileReco
8385
isNull(workspaceFiles.deletedAt)
8486
)
8587
)
88+
.orderBy(asc(workspaceFiles.uploadedAt), asc(workspaceFiles.id))
8689

8790
return rows.map(toWorkspaceFileRecord)
8891
} catch (err) {

0 commit comments

Comments
 (0)