fix(files): validate the revert revision before the no-op branch and omit an absent revision

Author: waleedlatif1

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

@@ -5321,15 +5321,8 @@
             "description": "Current public-share state, or null when the file has never been shared."
           },
           "revision": {
-            "anyOf": [
-              {
-                "type": "string"
-              },
-              {
-                "type": "null"
-              }
-            ],
-            "description": "Opaque token for the file's current content. Send it back as `expectedRevision` so a write or revert is refused when the content moved on. Null for a file with no recorded content version."
+            "description": "Opaque token for the file's current content. Send it back as `expectedRevision` so a write or revert is refused when the content moved on. Absent for a file with no recorded content version.",
+            "type": "string"
           },
           "currentVersion": {
             "type": "integer",
@@ -5351,7 +5344,6 @@
           "updatedAt",
           "deletedAt",
           "share",
-          "revision",
           "currentVersion"
         ],
         "additionalProperties": false,
@@ -5386,7 +5378,7 @@
               "deletedAt": null,
               "share": null,
               "currentVersion": 1,
-              "revision": "d2ZfNGtKOW1OMnBRN3JTOjIwMjYtMDEtMTVUMTA6MzA6MDAuMDAwWg"
+              "revision": "d2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTVUMTA6MzA6MDAuMDAwWg"
             }
           },
           {
@@ -5414,7 +5406,7 @@
                 "allowedEmails": []
               },
               "currentVersion": 3,
-              "revision": "d2ZfNGtKOW1OMnBRN3JTOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg"
+              "revision": "d2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg"
             }
           }
         ]

apps/sim/app/api/v2/files/[fileId]/metadata/route.ts

@@ -30,12 +30,15 @@ export const GET = defineV2JsonRoute({
     includeDeleted: query.scope === 'archived',
   }),
   useCase: readWorkspaceFileMetadataWithVersion,
-  present: async ({ file, share }) => ({
-    data: {
-      ...(await toV2File(file)),
-      share,
-      currentVersion: file.currentVersion,
-      revision: workspaceFileRevision(file),
-    },
-  }),
+  present: async ({ file, share }) => {
+    const revision = workspaceFileRevision(file)
+    return {
+      data: {
+        ...(await toV2File(file)),
+        share,
+        currentVersion: file.currentVersion,
+        ...(revision === null ? {} : { revision }),
+      },
+    }
+  },
 })

apps/sim/lib/api/contracts/v2/files.ts

@@ -158,9 +158,9 @@ export const v2FileMetadataSchema = v2FileSchema
       .describe('Current public-share state, or null when the file has never been shared.'),
     revision: z
       .string()
-      .nullable()
+      .optional()
       .describe(
-        "Opaque token for the file's current content. Send it back as `expectedRevision` so a write or revert is refused when the content moved on. Null for a file with no recorded content version."
+        "Opaque token for the file's current content. Send it back as `expectedRevision` so a write or revert is refused when the content moved on. Absent for a file with no recorded content version."
       ),
     currentVersion: versionNumberSchema.describe(
       'Version number of the current content. List File Versions returns the history; pass this as `expectedCurrentVersion` to revert only if nothing changed since.'

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

@@ -876,15 +876,15 @@ const declaredRoutes = [
               ...FILE_EXAMPLE,
               share: null,
               currentVersion: 1,
-              revision: 'd2ZfNGtKOW1OMnBRN3JTOjIwMjYtMDEtMTVUMTA6MzA6MDAuMDAwWg',
+              revision: 'd2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTVUMTA6MzA6MDAuMDAwWg',
             },
           },
           {
             data: {
               ...FILE_EXAMPLE,
               share: SHARE_EXAMPLE,
               currentVersion: 3,
-              revision: 'd2ZfNGtKOW1OMnBRN3JTOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg',
+              revision: 'd2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg',
             },
           },
         ]

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

@@ -211,8 +211,7 @@ describe('file version use cases', () => {
     })
 
     /** The revision guards content, so it catches an edit that folded into the current version. */
-    it('guards the revert with the revision the caller read', async () => {
-      const callerRevision = new Date('2026-01-02T00:00:00Z')
+    it('reverts when the revision still names the current content', async () => {
       mocks.getVersion.mockImplementation(async (_file: unknown, number: number) =>
         number === 2 ? version(2) : number === 4 ? version(4, { isCurrent: true }) : current
       )
@@ -223,15 +222,34 @@ describe('file version use cases', () => {
           fileId: 'file-1',
           assertedWorkspaceId: 'workspace-1',
           version: 2,
-          expectedRevision: workspaceFileRevision({ ...file, contentUpdatedAt: callerRevision }),
+          expectedRevision: workspaceFileRevision(file),
         },
       })
 
       expect(mocks.updateContent.mock.calls[0][5]).toMatchObject({
-        expectedUpdatedAt: callerRevision,
+        expectedUpdatedAt: file.contentUpdatedAt,
       })
     })
 
+    /** Reverting to the version that is already current must still honour a stale revision. */
+    it('refuses a stale revision even when the requested version is already current', async () => {
+      await expect(
+        revertWorkspaceFileVersion.execute({
+          principal,
+          input: {
+            fileId: 'file-1',
+            assertedWorkspaceId: 'workspace-1',
+            version: 3,
+            expectedRevision: workspaceFileRevision({
+              ...file,
+              contentUpdatedAt: new Date('2020-01-01T00:00:00Z'),
+            }),
+          },
+        })
+      ).rejects.toMatchObject({ code: 'conflict' })
+      expect(mocks.updateContent).not.toHaveBeenCalled()
+    })
+
     it('refuses a revision issued for a different file', async () => {
       await expect(
         revertWorkspaceFileVersion.execute({

apps/sim/lib/workspace-files/application/file-versions.ts

@@ -236,6 +236,22 @@ async function executeRevertWorkspaceFileVersion({
       `The current version is ${current.version}, not ${input.expectedCurrentVersion}`
     )
   }
+  /*
+   * Checked before the no-op branch below: a caller that named content which has since changed
+   * must hear about it, not be told there was nothing to do.
+   */
+  const expectedContentAt = input.expectedRevision
+    ? parseWorkspaceFileRevision(input.expectedRevision, context.fileId)
+    : undefined
+  if (
+    expectedContentAt &&
+    (file.contentUpdatedAt ?? file.updatedAt).getTime() !== expectedContentAt.getTime()
+  ) {
+    throw new OrchestrationError(
+      'conflict',
+      'The file changed since the revision you read; re-read it before reverting'
+    )
+  }
   const target = await loadVersion(file, input.version)
   if (target.isCurrent) {
     return { file, version: target, reverted: false, revertedFrom: current.version }
@@ -272,9 +288,7 @@ async function executeRevertWorkspaceFileVersion({
           source: 'revert',
           restoredFromVersion: target.version,
         }),
-        expectedUpdatedAt: input.expectedRevision
-          ? parseWorkspaceFileRevision(input.expectedRevision, context.fileId)
-          : (file.contentUpdatedAt ?? file.updatedAt),
+        expectedUpdatedAt: expectedContentAt ?? file.contentUpdatedAt ?? file.updatedAt,
         secretProvenancePolicy: { mode: 'reinstate', snapshot: provenance },
       }
     )

packages/sim-cli/src/generated/v2-api.ts

@@ -4162,7 +4162,7 @@ type GetFileResponseRef1 = {
   updatedAt: string
   deletedAt: string | null
   share: GetFileResponseRef0 | null
-  revision: string | null
+  revision?: string
   currentVersion: number
 }