fix(files): never pair a stale file record with a newer version number

Author: waleedlatif1

apps/docs/openapi-v2-files-audit.json

@@ -2068,7 +2068,7 @@
       "get": {
         "operationId": "getFile",
         "summary": "Get File Metadata",
-        "description": "Get file metadata, its public-share configuration, and the version number of its current content. The `share` field is null when the file has never been shared. `currentVersion` identifies the content in List File Versions and is the precondition Revert File Version accepts.\n\nOAuth scope: `api:read`.",
+        "description": "Get file metadata, its public-share configuration, and the version number of its current content. The `share` field is null when the file has never been shared. `currentVersion` identifies the content in List File Versions and is the precondition Revert File Version accepts. A file rewritten continuously while it is read returns `409`; retry.\n\nOAuth scope: `api:read`.",
         "x-sim-operation": "files.read_metadata",
         "x-oauth-scope": "api:read",
         "tags": ["Files"],
@@ -2145,6 +2145,9 @@
           "404": {
             "$ref": "#/components/responses/NotFound"
           },
+          "409": {
+            "$ref": "#/components/responses/Conflict"
+          },
           "429": {
             "$ref": "#/components/responses/RateLimited"
           },

apps/sim/lib/api/contracts/v2/openapi/files-audit.ts

@@ -848,8 +848,8 @@ const declaredRoutes = [
       operationId: 'getFile',
       summary: 'Get File Metadata',
       description:
-        'Get file metadata, its public-share configuration, and the version number of its current content. The `share` field is null when the file has never been shared. `currentVersion` identifies the content in List File Versions and is the precondition Revert File Version accepts.',
-      errors: RESOURCE_ERRORS,
+        'Get file metadata, its public-share configuration, and the version number of its current content. The `share` field is null when the file has never been shared. `currentVersion` identifies the content in List File Versions and is the precondition Revert File Version accepts. A file rewritten continuously while it is read returns `409`; retry.',
+      errors: RESOURCE_CONFLICT_ERRORS,
       success: { description: 'File metadata and public-share state.' },
     }),
     {

apps/sim/lib/api/mcp/generated/v2-operations.ts

@@ -1035,7 +1035,7 @@ export const V2_MCP_OPERATIONS = {
     contract: v2GetFileContract,
     summary: 'Get File Metadata',
     description:
-      'Get file metadata, its public-share configuration, and the version number of its current content. The `share` field is null when the file has never been shared. `currentVersion` identifies the content in List File Versions and is the precondition Revert File Version accepts.\n\nOAuth scope: `api:read`.',
+      'Get file metadata, its public-share configuration, and the version number of its current content. The `share` field is null when the file has never been shared. `currentVersion` identifies the content in List File Versions and is the precondition Revert File Version accepts. A file rewritten continuously while it is read returns `409`; retry.\n\nOAuth scope: `api:read`.',
     handler: () => import('@/app/api/v2/files/[fileId]/metadata/route').then((route) => route.GET),
   },
   getFileShare: {

apps/sim/lib/uploads/contexts/workspace/__integration__/file-versions.integration.ts

@@ -271,6 +271,34 @@ describe('workspace file version history in PostgreSQL', () => {
     expect(await objectExists(fixture.firstKey)).toBe(false)
   })
 
+  it('reports a stale record as unresolvable once a coalesced write released its key', async () => {
+    const fixture = await seedFile('original')
+    const write = { source: 'collab', authorUserId: fixture.aliceId } as const
+    await updateWorkspaceFileContent(
+      fixture.workspaceId,
+      fixture.fileId,
+      fixture.aliceId,
+      Buffer.from('draft one'),
+      undefined,
+      { version: write }
+    )
+    const stale = await getWorkspaceFile(fixture.workspaceId, fixture.fileId)
+    if (!stale) throw new Error('file missing')
+    await updateWorkspaceFileContent(
+      fixture.workspaceId,
+      fixture.fileId,
+      fixture.aliceId,
+      Buffer.from('draft two'),
+      undefined,
+      { version: write }
+    )
+    const fresh = await getWorkspaceFile(fixture.workspaceId, fixture.fileId)
+    if (!fresh) throw new Error('file missing')
+
+    expect(await getWorkspaceFileVersionNumberForRecord(stale)).toBeNull()
+    expect(await getWorkspaceFileVersionNumberForRecord(fresh)).toBe(2)
+  })
+
   it('never folds deliberate writes, and repoints the head for identical bytes', async () => {
     const fixture = await seedFile('original')
     const write = { source: 'api', authorUserId: fixture.aliceId } as const

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

@@ -498,21 +498,21 @@ export async function getCurrentWorkspaceFileVersion(
 }
 
 /**
- * The version number of the content a file record describes. Matched by the record's storage key, so
- * it stays exact even if a write committed after the record was read: that record's key still names
- * its own version row. A key with no row is either a file with no history (implicit version 1) or
- * bytes a collaborative write replaced within the same version, which keeps the head's number.
+ * The version number of the content a file record describes, matched by the record's storage key so
+ * it stays exact even if a write committed after the record was read. Returns null when the record
+ * is stale in a way its key cannot answer — a later write replaced those bytes within their version
+ * and released the key — so the caller re-reads the record rather than guess.
  */
 export async function getWorkspaceFileVersionNumberForRecord(
   file: Pick<WorkspaceFileVersionSubject, 'id' | 'key'>
-): Promise<number> {
+): Promise<number | null> {
   const [row] = await db
     .select({ version: workspaceFileVersion.version })
     .from(workspaceFileVersion)
     .where(and(eq(workspaceFileVersion.fileId, file.id), eq(workspaceFileVersion.key, file.key)))
     .limit(1)
   if (row) return row.version
-  return (await loadWorkspaceFileVersionHead(file.id))?.version ?? 1
+  return (await loadWorkspaceFileVersionHead(file.id)) ? null : 1
 }
 
 /** One version of a file, or null when it never existed or retention removed it. */

apps/sim/lib/workspace-files/application/read-workspace-file-metadata.test.ts

@@ -189,4 +189,30 @@ describe('readWorkspaceFileMetadataWithVersion', () => {
     ).resolves.toEqual({ file, share, currentVersion: 4 })
     expect(mocks.getVersionNumberForRecord).toHaveBeenCalledWith(file)
   })
+
+  it('re-reads a record whose bytes a concurrent write already replaced', async () => {
+    const rewritten = { ...file, key: 'workspace/ws/data-2.csv' }
+    mocks.getWorkspaceFile.mockResolvedValueOnce(file).mockResolvedValueOnce(rewritten)
+    mocks.getVersionNumberForRecord.mockResolvedValueOnce(null).mockResolvedValueOnce(5)
+
+    await expect(
+      readWorkspaceFileMetadataWithVersion.execute({
+        principal,
+        input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' },
+      })
+    ).resolves.toEqual({ file: rewritten, share, currentVersion: 5 })
+  })
+
+  it('answers a retryable conflict when every read is already stale', async () => {
+    mocks.getWorkspaceFile.mockResolvedValue(file)
+    mocks.getVersionNumberForRecord.mockResolvedValue(null)
+
+    await expect(
+      readWorkspaceFileMetadataWithVersion.execute({
+        principal,
+        input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1' },
+      })
+    ).rejects.toMatchObject({ code: 'conflict' })
+    expect(mocks.getVersionNumberForRecord).toHaveBeenCalledTimes(3)
+  })
 })

apps/sim/lib/workspace-files/application/read-workspace-file-metadata.ts

@@ -59,18 +59,29 @@ export const readWorkspaceFileMetadata = defineAuthorizedWorkspaceFileUseCase({
   execute: executeReadWorkspaceFileMetadata,
 })
 
+/** Reads of a record a concurrent write keeps replacing before the version is reported as a conflict. */
+const CURRENT_VERSION_READ_ATTEMPTS = 3
+
 /**
  * The same read plus the current version number, for the public metadata surface. Kept separate so
  * the many internal callers of {@link readWorkspaceFileMetadata} pay no extra query. The number is
  * resolved from the returned record's own storage key, so it always identifies the content that
- * record describes.
+ * record describes; a record whose bytes a concurrent write already replaced is read again, and a
+ * file rewritten on every attempt answers a retryable conflict rather than a mismatched version.
  */
 export const readWorkspaceFileMetadataWithVersion = defineAuthorizedWorkspaceFileUseCase({
   operation: fileOperations.readMetadata,
   resolveContext: ({ input }: { input: ReadWorkspaceFileMetadataInput }) =>
     resolveActiveWorkspaceFileContext(input),
   async execute(args): Promise<ReadWorkspaceFileMetadataWithVersionResult> {
-    const result = await executeReadWorkspaceFileMetadata(args)
-    return { ...result, currentVersion: await getWorkspaceFileVersionNumberForRecord(result.file) }
+    for (let attempt = 0; attempt < CURRENT_VERSION_READ_ATTEMPTS; attempt++) {
+      const result = await executeReadWorkspaceFileMetadata(args)
+      const currentVersion = await getWorkspaceFileVersionNumberForRecord(result.file)
+      if (currentVersion !== null) return { ...result, currentVersion }
+    }
+    throw new OrchestrationError(
+      'conflict',
+      'The file changed while it was being read; retry the request'
+    )
   },
 })