Skip to content

Commit 3ba5928

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 narrowed to the displayName index (other 23505s, e.g. key-active collisions from concurrent same-s3Key inserts, rethrow). Returns the assigned displayName so the read-hint string given to the model uses it. Resolver matches displayName for new rows, falls back to original_name for legacy NULL rows, and orders by uploaded_at DESC so legacy ambiguity resolves to the newest upload (which fixes the original prod symptom even pre-fix).
1 parent 288f023 commit 3ba5928

3 files changed

Lines changed: 10 additions & 22 deletions

File tree

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

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,7 @@ import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace/works
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-
*/
13+
/** VFS-visible name. Coalesces to originalName for legacy rows that predate displayName. */
1814
function vfsName(row: typeof workspaceFiles.$inferSelect): string {
1915
return row.displayName ?? row.originalName
2016
}

apps/sim/lib/uploads/contexts/workspace/track-chat-upload.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
77

88
vi.mock('@sim/db', () => dbChainMock)
99

10-
import { suffixedName, trackChatUpload } from './workspace-file-manager'
10+
import { CHAT_DISPLAY_NAME_INDEX, suffixedName, trackChatUpload } from './workspace-file-manager'
1111

1212
const CHAT_ID = '11111111-1111-1111-1111-111111111111'
1313
const WORKSPACE_ID = 'ws_1'
@@ -100,7 +100,7 @@ describe('trackChatUpload', () => {
100100
// 23505 from the partial unique index on (chat_id, display_name) — the case we retry.
101101
const displayNameCollision = Object.assign(new Error('duplicate key'), {
102102
code: '23505',
103-
constraint_name: 'workspace_files_chat_display_name_unique',
103+
constraint_name: CHAT_DISPLAY_NAME_INDEX,
104104
})
105105

106106
// Attempt 1: UPDATE finds no row (returning -> []), then INSERT throws displayName 23505.

apps/sim/lib/uploads/contexts/workspace/workspace-file-manager.ts

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -408,21 +408,18 @@ export async function registerUploadedWorkspaceFile(params: {
408408
}
409409

410410
/**
411-
* Inserts ` (n)` before the last extension, mirroring `withCopySuffix` but starting at n=1
412-
* for the second occurrence (so `image.png`, `image (2).png`, `image (3).png`).
413-
* Extensionless names get `name (n)`; dotfiles like `.env` are treated as extensionless.
411+
* Like `withCopySuffix` but with `n=1` meaning "no suffix" — used by retry loops where
412+
* the first attempt should try the original name (`image.png`, `image (2).png`, ...).
413+
* Exported for tests.
414414
*/
415415
export function suffixedName(name: string, n: number): string {
416-
if (n <= 1) return name
417-
const dot = name.lastIndexOf('.')
418-
if (dot <= 0 || dot === name.length - 1) return `${name} (${n})`
419-
return `${name.slice(0, dot)} (${n})${name.slice(dot)}`
416+
return n <= 1 ? name : withCopySuffix(name, n)
420417
}
421418

422419
const MAX_CHAT_DISPLAY_NAME_RETRIES = 1000
423420

424421
/** Postgres constraint name for the partial unique index on `(chat_id, display_name)`. */
425-
const CHAT_DISPLAY_NAME_INDEX = 'workspace_files_chat_display_name_unique'
422+
export const CHAT_DISPLAY_NAME_INDEX = 'workspace_files_chat_display_name_unique'
426423

427424
/**
428425
* Track a file that was already uploaded to workspace S3 as a chat-scoped upload.
@@ -485,13 +482,8 @@ export async function trackChatUpload(
485482
)
486483
return { displayName: candidate }
487484
} catch (error) {
488-
/**
489-
* Only retry on a collision against the chat displayName index. Any other 23505
490-
* (the active-key index from a concurrent insert with the same s3Key, or a
491-
* primary-key collision) means a different invariant is contested — bumping the
492-
* suffix would silently rename whichever row eventually owns the s3Key, including
493-
* a row another caller has already returned to its client.
494-
*/
485+
// Other 23505s (e.g. active-key collision from a racing same-s3Key insert) signal
486+
// a different invariant — retrying would silently rename a row another caller owns.
495487
if (
496488
getPostgresErrorCode(error) === '23505' &&
497489
getPostgresConstraintName(error) === CHAT_DISPLAY_NAME_INDEX

0 commit comments

Comments
 (0)