From a6cec8803fc1f20207df7bb72c56a15b7d990e6c Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 20 Sep 2026 21:18:05 -0700 Subject: [PATCH 1/2] fix(files,realtime): issue a revision on revert and stop resurrecting cleared cursors Follow-ups to review findings on the v0.8.48 release PR. - POST /api/v2/files/:id/versions/:version/revert now returns the revision naming the content the file holds after the revert. A revert consumes the caller's revision, so without it chaining a second conditional write needed a metadata re-read, and the gap between the two reopened the TOCTOU window the revision exists to close. - The presence roster merge now distinguishes an absent cursor from an explicit null. The fallback is load-bearing: the server rebuilds a socket's presence record on join, so a re-join broadcasts a roster with cursor omitted. But a null is a pointer the peer cleared, and coalescing both with ?? re-pinned a ghost cursor whenever the clearing cursor-update was missed. - Corrected two File block output descriptions that named operations the block does not have: revision claimed a get operation (this block has Get Content, which emits no revision) and lineCount named insert, which is an edit mode. Also collapses the five hand-rolled copies of the revision conditional spread into workspaceFileRevisionField, beside the token it describes. --- apps/docs/openapi-v2-files-audit.json | 7 +- .../api/v2/files/[fileId]/content/route.ts | 18 +- .../api/v2/files/[fileId]/metadata/route.ts | 21 +-- .../versions/[version]/revert/route.test.ts | 163 ++++++++++++++++++ .../versions/[version]/revert/route.ts | 13 +- .../providers/socket-presence-merge.test.ts | 60 +++++++ .../providers/socket-presence-merge.ts | 32 ++++ .../workspace/providers/socket-provider.tsx | 17 +- apps/sim/blocks/blocks/file.ts | 4 +- .../sim/lib/api/contracts/v2/file-versions.ts | 4 + apps/sim/lib/api/contracts/v2/files.ts | 2 +- .../api/contracts/v2/openapi/files-audit.ts | 1 + apps/sim/lib/internal/file/operations.ts | 19 +- .../application/file-revision.ts | 12 ++ .../application/file-versions.test.ts | 39 ++++- packages/sim-cli/src/generated/v2-api.ts | 1 + 16 files changed, 350 insertions(+), 63 deletions(-) create mode 100644 apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.test.ts create mode 100644 apps/sim/app/workspace/providers/socket-presence-merge.test.ts create mode 100644 apps/sim/app/workspace/providers/socket-presence-merge.ts diff --git a/apps/docs/openapi-v2-files-audit.json b/apps/docs/openapi-v2-files-audit.json index a1ce1ccd7f7..8fd997a4e9f 100644 --- a/apps/docs/openapi-v2-files-audit.json +++ b/apps/docs/openapi-v2-files-audit.json @@ -4923,6 +4923,10 @@ "version": { "description": "The current version of the file after the revert: a new `revert` version; the requested version when it was already current; or the unchanged current version when its content already matched the requested one.", "$ref": "#/components/schemas/V2FileVersion" + }, + "revision": { + "description": "Opaque token for the content the file holds after the revert — the one it just wrote, or the unchanged current content when `reverted` is false. Send it back as `expectedRevision` on the next write.", + "type": "string" } }, "required": ["reverted", "file", "version"], @@ -4976,7 +4980,8 @@ "createdAt": "2026-01-15T10:30:00Z", "updatedAt": "2026-01-15T10:30:00Z", "supersededAt": null - } + }, + "revision": "d2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg" } } ] diff --git a/apps/sim/app/api/v2/files/[fileId]/content/route.ts b/apps/sim/app/api/v2/files/[fileId]/content/route.ts index b2845c3a5e8..55c405528e1 100644 --- a/apps/sim/app/api/v2/files/[fileId]/content/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/content/route.ts @@ -5,7 +5,7 @@ import { import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' import { v2FileErrorPolicies } from '@/lib/workspace-files/api' import { editWorkspaceFileContent } from '@/lib/workspace-files/application/edit-workspace-file-content' -import { workspaceFileRevision } from '@/lib/workspace-files/application/file-revision' +import { workspaceFileRevisionField } from '@/lib/workspace-files/application/file-revision' import { fileOperations } from '@/lib/workspace-files/application/operations' import { admitUpdateWorkspaceFileContent, @@ -40,10 +40,9 @@ export const PUT = defineV2JsonRoute({ expectedRevision: body.expectedRevision, }), useCase: updateWorkspaceFileContent, - present: async ({ file }) => { - const revision = workspaceFileRevision(file) - return { data: { ...(await toV2File(file)), ...(revision === null ? {} : { revision }) } } - }, + present: async ({ file }) => ({ + data: { ...(await toV2File(file)), ...workspaceFileRevisionField(file) }, + }), }) /** @@ -78,10 +77,7 @@ export const PATCH = defineV2JsonRoute({ expectedRevision: body.expectedRevision, }), useCase: editWorkspaceFileContent, - present: async ({ file, lineCount }) => { - const revision = workspaceFileRevision(file) - return { - data: { file: await toV2File(file), lineCount, ...(revision === null ? {} : { revision }) }, - } - }, + present: async ({ file, lineCount }) => ({ + data: { file: await toV2File(file), lineCount, ...workspaceFileRevisionField(file) }, + }), }) diff --git a/apps/sim/app/api/v2/files/[fileId]/metadata/route.ts b/apps/sim/app/api/v2/files/[fileId]/metadata/route.ts index 415ccadad7e..02a57449037 100644 --- a/apps/sim/app/api/v2/files/[fileId]/metadata/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/metadata/route.ts @@ -1,7 +1,7 @@ import { v2GetFileContract } from '@/lib/api/contracts/v2/files' import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' import { v2FileErrorPolicies } from '@/lib/workspace-files/api' -import { workspaceFileRevision } from '@/lib/workspace-files/application/file-revision' +import { workspaceFileRevisionField } from '@/lib/workspace-files/application/file-revision' import { fileOperations } from '@/lib/workspace-files/application/operations' import { readWorkspaceFileMetadataWithVersion } from '@/lib/workspace-files/application/read-workspace-file-metadata' import { toV2File } from '@/app/api/v2/files/utils' @@ -30,15 +30,12 @@ export const GET = defineV2JsonRoute({ includeDeleted: query.scope === 'archived', }), useCase: readWorkspaceFileMetadataWithVersion, - present: async ({ file, share }) => { - const revision = workspaceFileRevision(file) - return { - data: { - ...(await toV2File(file)), - share, - currentVersion: file.currentVersion, - ...(revision === null ? {} : { revision }), - }, - } - }, + present: async ({ file, share }) => ({ + data: { + ...(await toV2File(file)), + share, + currentVersion: file.currentVersion, + ...workspaceFileRevisionField(file), + }, + }), }) diff --git a/apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.test.ts b/apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.test.ts new file mode 100644 index 00000000000..5de5043dff7 --- /dev/null +++ b/apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.test.ts @@ -0,0 +1,163 @@ +/** + * @vitest-environment node + */ +import { + V2_OPERATION_RATE_LIMIT_ALLOWED, + V2_PREAUTH_RATE_LIMIT_ALLOWED, + v2ApiKeyAuthModuleMock, + v2RateLimiterModuleMock, + v2RouteMocks, +} from '@sim/testing' +import { NextRequest } from 'next/server' +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const mocks = vi.hoisted(() => ({ + revertVersion: vi.fn(), + getUserEmailsByIds: vi.fn(), + findUserEmailsByIds: vi.fn(), +})) + +vi.mock('@/lib/workspace-files/application/file-versions', () => ({ + revertWorkspaceFileVersion: { + operation: { id: 'files.versions.revert', minimumRole: 'write', workspaceApiKey: 'allow' }, + execute: mocks.revertVersion, + }, +})) + +vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock) +vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock) + +vi.mock('@/lib/users/queries', () => ({ + getUserEmailsByIds: mocks.getUserEmailsByIds, + findUserEmailsByIds: mocks.findUserEmailsByIds, + requireResolvedUserEmail: (emails: Map, userId: string) => emails.get(userId)!, +})) + +import { workspaceFileRevision } from '@/lib/workspace-files/application/file-revision' +import { POST } from '@/app/api/v2/files/[fileId]/versions/[version]/revert/route' + +const WORKSPACE_ID = 'workspace-1' +const FILE_ID = 'wf_1' +const auth = { + principal: { + kind: 'workspace_api_key' as const, + workspaceId: WORKSPACE_ID, + keyId: 'key-1', + }, + rateLimitSubjectIds: ['api-key:key-1', `workspace:${WORKSPACE_ID}`] as const, + rateLimitSubscription: null, + keyType: 'workspace' as const, +} + +const record = { + id: FILE_ID, + workspaceId: WORKSPACE_ID, + name: 'data.csv', + key: 'workspace/ws/1-x-data.csv', + path: '/api/files/serve/x', + size: 8, + type: 'text/csv', + uploadedBy: 'user-1', + folderId: null, + uploadedAt: new Date('2024-01-01T00:00:00Z'), + updatedAt: new Date('2024-01-03T00:00:00Z'), + contentUpdatedAt: new Date('2024-01-04T00:00:00Z'), +} + +const versionRecord = { + fileId: FILE_ID, + version: 4, + isCurrent: true, + size: 8, + contentType: 'text/csv', + source: 'revert' as const, + authorUserIds: ['user-1'], + restoredFromVersion: 2, + createdAt: new Date('2024-01-04T00:00:00Z'), + updatedAt: new Date('2024-01-04T00:00:00Z'), + supersededAt: null, +} + +const callRevert = (body: unknown) => + POST( + new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}/versions/2/revert`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(body), + }), + { params: Promise.resolve({ fileId: FILE_ID, version: '2' }) } + ) + +describe('POST /api/v2/files/[fileId]/versions/[version]/revert', () => { + beforeEach(() => { + vi.clearAllMocks() + v2RouteMocks.authenticate.mockResolvedValue(auth) + v2RouteMocks.preauthRate.mockResolvedValue(V2_PREAUTH_RATE_LIMIT_ALLOWED) + v2RouteMocks.operationRate.mockResolvedValue(V2_OPERATION_RATE_LIMIT_ALLOWED) + mocks.revertVersion.mockResolvedValue({ + file: record, + version: versionRecord, + reverted: true, + revertedFrom: 3, + }) + mocks.getUserEmailsByIds.mockResolvedValue(new Map([['user-1', 'ada@example.com']])) + mocks.findUserEmailsByIds.mockResolvedValue(new Map([['user-1', 'ada@example.com']])) + }) + + /** + * A revert consumes the caller's revision, so the response has to issue its replacement — + * otherwise chaining a second conditional write needs a metadata re-read, and the window + * between the two is exactly what the revision is meant to close. + */ + it('returns the revision naming the content the revert produced', async () => { + const expectedRevision = workspaceFileRevision(record)! + + const response = await callRevert({ workspaceId: WORKSPACE_ID, expectedRevision }) + + expect(response.status).toBe(200) + const body = await response.json() + expect(body.data.reverted).toBe(true) + expect(body.data.revision).toBe(expectedRevision) + expect(mocks.revertVersion).toHaveBeenCalledWith( + expect.objectContaining({ + input: expect.objectContaining({ + fileId: FILE_ID, + assertedWorkspaceId: WORKSPACE_ID, + version: 2, + expectedRevision, + }), + }) + ) + }) + + it('returns the current content revision when the version was already current', async () => { + mocks.revertVersion.mockResolvedValue({ + file: record, + version: { ...versionRecord, version: 3, source: 'api', restoredFromVersion: null }, + reverted: false, + revertedFrom: 3, + }) + + const body = await (await callRevert({ workspaceId: WORKSPACE_ID })).json() + + expect(body.data.reverted).toBe(false) + expect(body.data.revision).toBe(workspaceFileRevision(record)) + }) + + it('forwards the caller revision precondition to the use case', async () => { + const expectedRevision = workspaceFileRevision(record)! + + await callRevert({ workspaceId: WORKSPACE_ID, expectedRevision }) + + expect(mocks.revertVersion).toHaveBeenCalledWith( + expect.objectContaining({ + input: expect.objectContaining({ + fileId: FILE_ID, + assertedWorkspaceId: WORKSPACE_ID, + version: 2, + expectedRevision, + }), + }) + ) + }) +}) diff --git a/apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.ts b/apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.ts index 91a6c26cfb3..672b4150354 100644 --- a/apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.ts +++ b/apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.ts @@ -1,6 +1,7 @@ import { v2RevertFileVersionContract } from '@/lib/api/contracts/v2/file-versions' import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes' import { v2FileErrorPolicies } from '@/lib/workspace-files/api' +import { workspaceFileRevisionField } from '@/lib/workspace-files/application/file-revision' import { revertWorkspaceFileVersion } from '@/lib/workspace-files/application/file-versions' import { fileOperations } from '@/lib/workspace-files/application/operations' import { toV2File, toV2FileVersion } from '@/app/api/v2/files/utils' @@ -13,6 +14,9 @@ export const revalidate = 0 * * Writes the version's bytes as a new version, so the revert can itself be reverted. Reverting to * the current version is a no-op that reports `reverted: false`. + * + * A revert invalidates the revision the caller guarded it with, so the response carries the one + * naming the content the file now holds. */ export const POST = defineV2JsonRoute({ contract: v2RevertFileVersionContract, @@ -30,6 +34,13 @@ export const POST = defineV2JsonRoute({ useCase: revertWorkspaceFileVersion, present: async ({ file, version, reverted }) => { const [v2File, v2Version] = await Promise.all([toV2File(file), toV2FileVersion(version)]) - return { data: { reverted, file: v2File, version: v2Version } } + return { + data: { + reverted, + file: v2File, + version: v2Version, + ...workspaceFileRevisionField(file), + }, + } }, }) diff --git a/apps/sim/app/workspace/providers/socket-presence-merge.test.ts b/apps/sim/app/workspace/providers/socket-presence-merge.test.ts new file mode 100644 index 00000000000..3b032d33e63 --- /dev/null +++ b/apps/sim/app/workspace/providers/socket-presence-merge.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, it } from 'vitest' +import { mergePresenceRoster } from '@/app/workspace/providers/socket-presence-merge' +import type { PresenceUser } from '@/stores/presence/types' + +function peer(overrides: Partial = {}): PresenceUser { + return { + socketId: 'socket-1', + userId: 'user-1', + userName: 'Ada', + ...overrides, + } +} + +describe('mergePresenceRoster', () => { + it('clears the pointer when the roster carries an explicit null cursor', () => { + const previous = [peer({ cursor: { x: 10, y: 20 } })] + + const merged = mergePresenceRoster(previous, [peer({ cursor: null })]) + + expect(merged[0].cursor).toBeNull() + }) + + it('keeps the known pointer when the roster omits the cursor', () => { + const previous = [peer({ cursor: { x: 10, y: 20 } })] + + const merged = mergePresenceRoster(previous, [peer()]) + + expect(merged[0].cursor).toEqual({ x: 10, y: 20 }) + }) + + it('keeps the known selection when the roster omits it', () => { + const previous = [peer({ selection: { type: 'block', id: 'block-1' } })] + + const merged = mergePresenceRoster(previous, [peer()]) + + expect(merged[0].selection).toEqual({ type: 'block', id: 'block-1' }) + }) + + it('applies a cleared selection, which the wire spells as type none', () => { + const previous = [peer({ selection: { type: 'block', id: 'block-1' } })] + + const merged = mergePresenceRoster(previous, [peer({ selection: { type: 'none' } })]) + + expect(merged[0].selection).toEqual({ type: 'none' }) + }) + + it('passes through a peer it has no previous presence for', () => { + const joining = peer({ socketId: 'socket-2', userId: 'user-2', cursor: { x: 1, y: 2 } }) + + expect(mergePresenceRoster([], [joining])).toEqual([joining]) + }) + + it('drops peers the roster no longer lists', () => { + const previous = [peer(), peer({ socketId: 'socket-2', userId: 'user-2' })] + + const merged = mergePresenceRoster(previous, [peer()]) + + expect(merged.map((user) => user.socketId)).toEqual(['socket-1']) + }) +}) diff --git a/apps/sim/app/workspace/providers/socket-presence-merge.ts b/apps/sim/app/workspace/providers/socket-presence-merge.ts new file mode 100644 index 00000000000..08af5259cdc --- /dev/null +++ b/apps/sim/app/workspace/providers/socket-presence-merge.ts @@ -0,0 +1,32 @@ +import type { PresenceUser } from '@/stores/presence/types' + +/** + * Folds a `presence-update` roster over the presence already held for each socket. + * + * The server rebuilds a socket's presence record from scratch when it joins a room, so a re-join + * of the same workflow broadcasts a roster whose `cursor` and `selection` are simply absent. The + * fields are carried over rather than blanked, which is what keeps a peer's pointer from + * flickering on every re-join. + * + * A `null` `cursor` is therefore not the same as an absent one: it is a pointer the peer + * explicitly cleared on leaving the canvas. Coalescing the two with `??` would resurrect a stale + * pointer whenever the clearing `cursor-update` was missed — dropped by the visibility gate + * during a join, or by a rejoin that never refreshed the roster. `selection` needs no such + * split: a cleared selection is `{ type: 'none' }`, and the wire type admits no `null`. + */ +export function mergePresenceRoster( + previous: PresenceUser[], + incoming: PresenceUser[] +): PresenceUser[] { + const previousBySocketId = new Map(previous.map((user) => [user.socketId, user])) + + return incoming.map((user) => { + const existing = previousBySocketId.get(user.socketId) + if (!existing) return user + return { + ...user, + cursor: user.cursor === undefined ? existing.cursor : user.cursor, + selection: user.selection ?? existing.selection, + } + }) +} diff --git a/apps/sim/app/workspace/providers/socket-provider.tsx b/apps/sim/app/workspace/providers/socket-provider.tsx index e5587df0437..54bde7f3978 100644 --- a/apps/sim/app/workspace/providers/socket-provider.tsx +++ b/apps/sim/app/workspace/providers/socket-provider.tsx @@ -41,6 +41,7 @@ import { isSocketWorkflowVisible, resolveSocketWorkflowTarget, } from '@/app/workspace/providers/socket-join-target' +import { mergePresenceRoster } from '@/app/workspace/providers/socket-presence-merge' import { refreshSessionQuery } from '@/hooks/queries/session' import { useOperationQueueStore } from '@/stores/operation-queue/store' import type { @@ -600,21 +601,7 @@ export function SocketProvider({ children, user }: SocketProviderProps) { return } - updatePresenceUsers((prev) => { - const prevMap = new Map(prev.map((u) => [u.socketId, u])) - - return users.map((user) => { - const existing = prevMap.get(user.socketId) - if (existing) { - return { - ...user, - cursor: user.cursor ?? existing.cursor, - selection: user.selection ?? existing.selection, - } - } - return user - }) - }) + updatePresenceUsers((prev) => mergePresenceRoster(prev, users)) }) socketInstance.on('join-workflow-success', ({ workflowId, presenceUsers }) => { diff --git a/apps/sim/blocks/blocks/file.ts b/apps/sim/blocks/blocks/file.ts index ab69f7a21f4..ffb4fa9972a 100644 --- a/apps/sim/blocks/blocks/file.ts +++ b/apps/sim/blocks/blocks/file.ts @@ -2348,7 +2348,7 @@ export const FileV5Block: BlockConfig = { }, lineCount: { type: 'number', - description: 'Lines in the file after the change (edit, insert)', + description: 'Lines in the file after the change (edit)', }, version: { type: 'number', @@ -2357,7 +2357,7 @@ export const FileV5Block: BlockConfig = { revision: { type: 'string', description: - 'Opaque token for the content read or written, sent back as expectedRevision to make a later write conditional (get, write, append, edit)', + 'Opaque token for the content a write recorded, accepted as expectedRevision by the write and edit tools to make a later write conditional (write, append, edit)', }, results: { type: 'array', diff --git a/apps/sim/lib/api/contracts/v2/file-versions.ts b/apps/sim/lib/api/contracts/v2/file-versions.ts index 723aec41521..153dc6d0a85 100644 --- a/apps/sim/lib/api/contracts/v2/file-versions.ts +++ b/apps/sim/lib/api/contracts/v2/file-versions.ts @@ -12,6 +12,7 @@ import { v2FileTextSchema, v2FileWorkspaceQuerySchema, v2ReadFileTextQuerySchema, + writtenFileRevisionSchema, } from '@/lib/api/contracts/v2/files' import { v2CursorListResponse, @@ -154,6 +155,9 @@ export const v2RevertFileVersionResultSchema = z version: v2FileVersionSchema.describe( 'The current version of the file after the revert: a new `revert` version; the requested version when it was already current; or the unchanged current version when its content already matched the requested one.' ), + revision: writtenFileRevisionSchema.describe( + 'Opaque token for the content the file holds after the revert — the one it just wrote, or the unchanged current content when `reverted` is false. Send it back as `expectedRevision` on the next write.' + ), }) .strict() .meta({ diff --git a/apps/sim/lib/api/contracts/v2/files.ts b/apps/sim/lib/api/contracts/v2/files.ts index 5ad8949b2cb..5c9b601dc9e 100644 --- a/apps/sim/lib/api/contracts/v2/files.ts +++ b/apps/sim/lib/api/contracts/v2/files.ts @@ -72,7 +72,7 @@ import { FILE_SEARCH_MODES } from '@/lib/workspace-files/search/pattern' * and workflow writes fold into the current version rather than adding one. */ /** The token naming the content a write produced, for the caller's next conditional write. */ -const writtenFileRevisionSchema = z +export const writtenFileRevisionSchema = z .string() .optional() .describe( diff --git a/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts b/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts index fd90e6854d4..178b860ca32 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts @@ -644,6 +644,7 @@ const declaredRoutes = [ source: 'revert', restoredFromVersion: 3, }, + revision: 'd2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg', }, }, ] diff --git a/apps/sim/lib/internal/file/operations.ts b/apps/sim/lib/internal/file/operations.ts index cbe74f50d33..59489c2cb3e 100644 --- a/apps/sim/lib/internal/file/operations.ts +++ b/apps/sim/lib/internal/file/operations.ts @@ -71,14 +71,7 @@ import { createWorkspaceFileFromBuffer, } from '@/lib/workspace-files/application/create-workspace-file' import { editWorkspaceFileContent } from '@/lib/workspace-files/application/edit-workspace-file-content' -import { workspaceFileRevision } from '@/lib/workspace-files/application/file-revision' - -/** The revision a response advertises, omitted for a record that cannot name its content. */ -function revisionField(file: Parameters[0]) { - const revision = workspaceFileRevision(file) - return revision === null ? {} : { revision } -} - +import { workspaceFileRevisionField } from '@/lib/workspace-files/application/file-revision' import { listWorkspaceFilesInFolderScope, queryWorkspaceFilePage, @@ -1066,7 +1059,7 @@ export async function executeFileManageOperation( data: { file: workspaceFileToUserFile(file), /** The token a conditional write sends back; see `expectedRevision`. */ - ...revisionField(file), + ...workspaceFileRevisionField(file), }, }) } @@ -1428,7 +1421,7 @@ export async function executeFileManageOperation( size: overwritten.size, url: ensureAbsoluteUrl(overwritten.url ?? overwritten.path), version: overwritten.currentVersion, - ...revisionField(overwritten), + ...workspaceFileRevisionField(overwritten), }, }) } @@ -1480,7 +1473,7 @@ export async function executeFileManageOperation( url: ensureAbsoluteUrl(result.file.url ?? result.file.path), /** A file created with its content has no history yet, so those bytes are version 1. */ version: INITIAL_WORKSPACE_FILE_VERSION, - ...revisionField(result.file), + ...workspaceFileRevisionField(result.file), }, }) } @@ -1681,7 +1674,7 @@ export async function executeFileManageOperation( size: fileBuffer.length, url: ensureAbsoluteUrl(existing.path), version: appended.currentVersion, - ...revisionField(appended), + ...workspaceFileRevisionField(appended), }, }) } finally { @@ -1799,7 +1792,7 @@ export async function executeFileManageOperation( size: file.size, lineCount, version: file.currentVersion, - ...revisionField(file), + ...workspaceFileRevisionField(file), }, }) } diff --git a/apps/sim/lib/workspace-files/application/file-revision.ts b/apps/sim/lib/workspace-files/application/file-revision.ts index a2697ccd50d..87485581748 100644 --- a/apps/sim/lib/workspace-files/application/file-revision.ts +++ b/apps/sim/lib/workspace-files/application/file-revision.ts @@ -25,6 +25,18 @@ export function workspaceFileRevision( return Buffer.from(`${file.id}:${content.toISOString()}`).toString('base64url') } +/** + * The `revision` a response advertises, spread into the body. A record that cannot name its + * content contributes no key rather than a null one, which is the omission every surface's + * response schema declares. + */ +export function workspaceFileRevisionField(file: Parameters[0]): { + revision?: string +} { + const revision = workspaceFileRevision(file) + return revision === null ? {} : { revision } +} + /** * Reads back a revision this surface issued for `fileId`, as the content version to guard the * write with. A token for another file, or one this surface never issued, is refused rather than diff --git a/apps/sim/lib/workspace-files/application/file-versions.test.ts b/apps/sim/lib/workspace-files/application/file-versions.test.ts index ee9f402b527..1e26135639e 100644 --- a/apps/sim/lib/workspace-files/application/file-versions.test.ts +++ b/apps/sim/lib/workspace-files/application/file-versions.test.ts @@ -115,6 +115,14 @@ function version(number: number, overrides: Record = {}) { const current = version(3, { key: file.key, isCurrent: true, supersededAt: null }) +/** After a revert of v2: v2 is the source, v4 the new current version the write recorded. */ +const getVersionAfterRevert = async (_file: unknown, number: number) => + number === 2 + ? version(2) + : number === 4 + ? version(4, { source: 'revert', restoredFromVersion: 2, isCurrent: true }) + : current + function objectMissing() { return Object.assign(new Error('Failed to download file: missing'), { cause: Object.assign(new Error('missing'), { name: 'NoSuchKey' }), @@ -139,13 +147,7 @@ describe('file version use cases', () => { describe('revertWorkspaceFileVersion', () => { it('writes the version as a new revert version carrying its provenance snapshot', async () => { - mocks.getVersion.mockImplementation(async (_file: unknown, number: number) => - number === 2 - ? version(2) - : number === 4 - ? version(4, { source: 'revert', restoredFromVersion: 2, isCurrent: true }) - : current - ) + mocks.getVersion.mockImplementation(getVersionAfterRevert) const result = await revertWorkspaceFileVersion.execute({ principal, @@ -210,6 +212,29 @@ describe('file version use cases', () => { expect(mocks.notify).not.toHaveBeenCalled() }) + /** + * The surface presents `workspaceFileRevision(result.file)`, so the record has to be the one + * the write produced — a pre-write record would hand the caller a revision their next + * conditional write is guaranteed to fail on. + */ + it('returns the record the write produced, so its revision names the new content', async () => { + mocks.getVersion.mockImplementation(getVersionAfterRevert) + const contentUpdatedAt = new Date('2026-01-04T00:00:00Z') + mocks.updateContent.mockResolvedValue({ + ...file, + key: 'new-key', + currentVersion: 4, + contentUpdatedAt, + }) + + const result = await revertWorkspaceFileVersion.execute({ + principal, + input: { fileId: 'file-1', assertedWorkspaceId: 'workspace-1', version: 2 }, + }) + + expect(result.file.contentUpdatedAt).toEqual(contentUpdatedAt) + }) + /** The revision guards content, so it catches an edit that folded into the current version. */ it('reverts when the revision still names the current content', async () => { mocks.getVersion.mockImplementation(async (_file: unknown, number: number) => diff --git a/packages/sim-cli/src/generated/v2-api.ts b/packages/sim-cli/src/generated/v2-api.ts index ae7fbed6845..dc8616b41b2 100644 --- a/packages/sim-cli/src/generated/v2-api.ts +++ b/packages/sim-cli/src/generated/v2-api.ts @@ -9355,6 +9355,7 @@ type RevertFileVersionResponseRef2 = { reverted: boolean file: RevertFileVersionResponseRef0 version: RevertFileVersionResponseRef1 + revision?: string } export type RevertFileVersionResponse = { From d2e1475f4d11386d6383dd6e283aeb8d07d0b0a7 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 20 Sep 2026 21:28:54 -0700 Subject: [PATCH 2/2] fix(docs): match the revert example revision to its own record timestamp The revert response example carried a revision token encoding 2026-01-16T09:12:00.000Z while its file and version showed 2026-01-15T10:30:00Z. A revision is base64url(fileId:contentUpdatedAt), so the token has to name the record it ships with; every other example in the document already pairs them, and the one using the later timestamp overrides updatedAt to match. Echoing the old example back as expectedRevision would have named content the response never described. --- apps/docs/openapi-v2-files-audit.json | 2 +- apps/sim/lib/api/contracts/v2/openapi/files-audit.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/docs/openapi-v2-files-audit.json b/apps/docs/openapi-v2-files-audit.json index 8fd997a4e9f..77844ff2cbc 100644 --- a/apps/docs/openapi-v2-files-audit.json +++ b/apps/docs/openapi-v2-files-audit.json @@ -4981,7 +4981,7 @@ "updatedAt": "2026-01-15T10:30:00Z", "supersededAt": null }, - "revision": "d2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg" + "revision": "d2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTVUMTA6MzA6MDAuMDAwWg" } } ] diff --git a/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts b/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts index 178b860ca32..36858773bad 100644 --- a/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts +++ b/apps/sim/lib/api/contracts/v2/openapi/files-audit.ts @@ -644,7 +644,7 @@ const declaredRoutes = [ source: 'revert', restoredFromVersion: 3, }, - revision: 'd2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg', + revision: 'd2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTVUMTA6MzA6MDAuMDAwWg', }, }, ]