Skip to content

Commit 3f4c101

Browse files
authored
fix(files,realtime): issue a revision on revert and stop resurrecting cleared cursors (#8098)
* 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. * 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.
1 parent 081dd7c commit 3f4c101

16 files changed

Lines changed: 350 additions & 63 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4923,6 +4923,10 @@
49234923
"version": {
49244924
"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.",
49254925
"$ref": "#/components/schemas/V2FileVersion"
4926+
},
4927+
"revision": {
4928+
"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.",
4929+
"type": "string"
49264930
}
49274931
},
49284932
"required": ["reverted", "file", "version"],
@@ -4976,7 +4980,8 @@
49764980
"createdAt": "2026-01-15T10:30:00Z",
49774981
"updatedAt": "2026-01-15T10:30:00Z",
49784982
"supersededAt": null
4979-
}
4983+
},
4984+
"revision": "d2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTVUMTA6MzA6MDAuMDAwWg"
49804985
}
49814986
}
49824987
]

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

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
66
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
77
import { editWorkspaceFileContent } from '@/lib/workspace-files/application/edit-workspace-file-content'
8-
import { workspaceFileRevision } from '@/lib/workspace-files/application/file-revision'
8+
import { workspaceFileRevisionField } from '@/lib/workspace-files/application/file-revision'
99
import { fileOperations } from '@/lib/workspace-files/application/operations'
1010
import {
1111
admitUpdateWorkspaceFileContent,
@@ -40,10 +40,9 @@ export const PUT = defineV2JsonRoute({
4040
expectedRevision: body.expectedRevision,
4141
}),
4242
useCase: updateWorkspaceFileContent,
43-
present: async ({ file }) => {
44-
const revision = workspaceFileRevision(file)
45-
return { data: { ...(await toV2File(file)), ...(revision === null ? {} : { revision }) } }
46-
},
43+
present: async ({ file }) => ({
44+
data: { ...(await toV2File(file)), ...workspaceFileRevisionField(file) },
45+
}),
4746
})
4847

4948
/**
@@ -78,10 +77,7 @@ export const PATCH = defineV2JsonRoute({
7877
expectedRevision: body.expectedRevision,
7978
}),
8079
useCase: editWorkspaceFileContent,
81-
present: async ({ file, lineCount }) => {
82-
const revision = workspaceFileRevision(file)
83-
return {
84-
data: { file: await toV2File(file), lineCount, ...(revision === null ? {} : { revision }) },
85-
}
86-
},
80+
present: async ({ file, lineCount }) => ({
81+
data: { file: await toV2File(file), lineCount, ...workspaceFileRevisionField(file) },
82+
}),
8783
})

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

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { v2GetFileContract } from '@/lib/api/contracts/v2/files'
22
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
33
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
4-
import { workspaceFileRevision } from '@/lib/workspace-files/application/file-revision'
4+
import { workspaceFileRevisionField } from '@/lib/workspace-files/application/file-revision'
55
import { fileOperations } from '@/lib/workspace-files/application/operations'
66
import { readWorkspaceFileMetadataWithVersion } from '@/lib/workspace-files/application/read-workspace-file-metadata'
77
import { toV2File } from '@/app/api/v2/files/utils'
@@ -30,15 +30,12 @@ export const GET = defineV2JsonRoute({
3030
includeDeleted: query.scope === 'archived',
3131
}),
3232
useCase: readWorkspaceFileMetadataWithVersion,
33-
present: async ({ file, share }) => {
34-
const revision = workspaceFileRevision(file)
35-
return {
36-
data: {
37-
...(await toV2File(file)),
38-
share,
39-
currentVersion: file.currentVersion,
40-
...(revision === null ? {} : { revision }),
41-
},
42-
}
43-
},
33+
present: async ({ file, share }) => ({
34+
data: {
35+
...(await toV2File(file)),
36+
share,
37+
currentVersion: file.currentVersion,
38+
...workspaceFileRevisionField(file),
39+
},
40+
}),
4441
})
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import {
5+
V2_OPERATION_RATE_LIMIT_ALLOWED,
6+
V2_PREAUTH_RATE_LIMIT_ALLOWED,
7+
v2ApiKeyAuthModuleMock,
8+
v2RateLimiterModuleMock,
9+
v2RouteMocks,
10+
} from '@sim/testing'
11+
import { NextRequest } from 'next/server'
12+
import { beforeEach, describe, expect, it, vi } from 'vitest'
13+
14+
const mocks = vi.hoisted(() => ({
15+
revertVersion: vi.fn(),
16+
getUserEmailsByIds: vi.fn(),
17+
findUserEmailsByIds: vi.fn(),
18+
}))
19+
20+
vi.mock('@/lib/workspace-files/application/file-versions', () => ({
21+
revertWorkspaceFileVersion: {
22+
operation: { id: 'files.versions.revert', minimumRole: 'write', workspaceApiKey: 'allow' },
23+
execute: mocks.revertVersion,
24+
},
25+
}))
26+
27+
vi.mock('@/lib/api/server/routes/v2-api-key-auth', () => v2ApiKeyAuthModuleMock)
28+
vi.mock('@/lib/core/rate-limiter', () => v2RateLimiterModuleMock)
29+
30+
vi.mock('@/lib/users/queries', () => ({
31+
getUserEmailsByIds: mocks.getUserEmailsByIds,
32+
findUserEmailsByIds: mocks.findUserEmailsByIds,
33+
requireResolvedUserEmail: (emails: Map<string, string>, userId: string) => emails.get(userId)!,
34+
}))
35+
36+
import { workspaceFileRevision } from '@/lib/workspace-files/application/file-revision'
37+
import { POST } from '@/app/api/v2/files/[fileId]/versions/[version]/revert/route'
38+
39+
const WORKSPACE_ID = 'workspace-1'
40+
const FILE_ID = 'wf_1'
41+
const auth = {
42+
principal: {
43+
kind: 'workspace_api_key' as const,
44+
workspaceId: WORKSPACE_ID,
45+
keyId: 'key-1',
46+
},
47+
rateLimitSubjectIds: ['api-key:key-1', `workspace:${WORKSPACE_ID}`] as const,
48+
rateLimitSubscription: null,
49+
keyType: 'workspace' as const,
50+
}
51+
52+
const record = {
53+
id: FILE_ID,
54+
workspaceId: WORKSPACE_ID,
55+
name: 'data.csv',
56+
key: 'workspace/ws/1-x-data.csv',
57+
path: '/api/files/serve/x',
58+
size: 8,
59+
type: 'text/csv',
60+
uploadedBy: 'user-1',
61+
folderId: null,
62+
uploadedAt: new Date('2024-01-01T00:00:00Z'),
63+
updatedAt: new Date('2024-01-03T00:00:00Z'),
64+
contentUpdatedAt: new Date('2024-01-04T00:00:00Z'),
65+
}
66+
67+
const versionRecord = {
68+
fileId: FILE_ID,
69+
version: 4,
70+
isCurrent: true,
71+
size: 8,
72+
contentType: 'text/csv',
73+
source: 'revert' as const,
74+
authorUserIds: ['user-1'],
75+
restoredFromVersion: 2,
76+
createdAt: new Date('2024-01-04T00:00:00Z'),
77+
updatedAt: new Date('2024-01-04T00:00:00Z'),
78+
supersededAt: null,
79+
}
80+
81+
const callRevert = (body: unknown) =>
82+
POST(
83+
new NextRequest(`http://localhost:3000/api/v2/files/${FILE_ID}/versions/2/revert`, {
84+
method: 'POST',
85+
headers: { 'Content-Type': 'application/json' },
86+
body: JSON.stringify(body),
87+
}),
88+
{ params: Promise.resolve({ fileId: FILE_ID, version: '2' }) }
89+
)
90+
91+
describe('POST /api/v2/files/[fileId]/versions/[version]/revert', () => {
92+
beforeEach(() => {
93+
vi.clearAllMocks()
94+
v2RouteMocks.authenticate.mockResolvedValue(auth)
95+
v2RouteMocks.preauthRate.mockResolvedValue(V2_PREAUTH_RATE_LIMIT_ALLOWED)
96+
v2RouteMocks.operationRate.mockResolvedValue(V2_OPERATION_RATE_LIMIT_ALLOWED)
97+
mocks.revertVersion.mockResolvedValue({
98+
file: record,
99+
version: versionRecord,
100+
reverted: true,
101+
revertedFrom: 3,
102+
})
103+
mocks.getUserEmailsByIds.mockResolvedValue(new Map([['user-1', 'ada@example.com']]))
104+
mocks.findUserEmailsByIds.mockResolvedValue(new Map([['user-1', 'ada@example.com']]))
105+
})
106+
107+
/**
108+
* A revert consumes the caller's revision, so the response has to issue its replacement —
109+
* otherwise chaining a second conditional write needs a metadata re-read, and the window
110+
* between the two is exactly what the revision is meant to close.
111+
*/
112+
it('returns the revision naming the content the revert produced', async () => {
113+
const expectedRevision = workspaceFileRevision(record)!
114+
115+
const response = await callRevert({ workspaceId: WORKSPACE_ID, expectedRevision })
116+
117+
expect(response.status).toBe(200)
118+
const body = await response.json()
119+
expect(body.data.reverted).toBe(true)
120+
expect(body.data.revision).toBe(expectedRevision)
121+
expect(mocks.revertVersion).toHaveBeenCalledWith(
122+
expect.objectContaining({
123+
input: expect.objectContaining({
124+
fileId: FILE_ID,
125+
assertedWorkspaceId: WORKSPACE_ID,
126+
version: 2,
127+
expectedRevision,
128+
}),
129+
})
130+
)
131+
})
132+
133+
it('returns the current content revision when the version was already current', async () => {
134+
mocks.revertVersion.mockResolvedValue({
135+
file: record,
136+
version: { ...versionRecord, version: 3, source: 'api', restoredFromVersion: null },
137+
reverted: false,
138+
revertedFrom: 3,
139+
})
140+
141+
const body = await (await callRevert({ workspaceId: WORKSPACE_ID })).json()
142+
143+
expect(body.data.reverted).toBe(false)
144+
expect(body.data.revision).toBe(workspaceFileRevision(record))
145+
})
146+
147+
it('forwards the caller revision precondition to the use case', async () => {
148+
const expectedRevision = workspaceFileRevision(record)!
149+
150+
await callRevert({ workspaceId: WORKSPACE_ID, expectedRevision })
151+
152+
expect(mocks.revertVersion).toHaveBeenCalledWith(
153+
expect.objectContaining({
154+
input: expect.objectContaining({
155+
fileId: FILE_ID,
156+
assertedWorkspaceId: WORKSPACE_ID,
157+
version: 2,
158+
expectedRevision,
159+
}),
160+
})
161+
)
162+
})
163+
})

‎apps/sim/app/api/v2/files/[fileId]/versions/[version]/revert/route.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { v2RevertFileVersionContract } from '@/lib/api/contracts/v2/file-versions'
22
import { defineV2JsonRoute, v2ApiKeyAuth, v2RateLimits } from '@/lib/api/server/routes'
33
import { v2FileErrorPolicies } from '@/lib/workspace-files/api'
4+
import { workspaceFileRevisionField } from '@/lib/workspace-files/application/file-revision'
45
import { revertWorkspaceFileVersion } from '@/lib/workspace-files/application/file-versions'
56
import { fileOperations } from '@/lib/workspace-files/application/operations'
67
import { toV2File, toV2FileVersion } from '@/app/api/v2/files/utils'
@@ -13,6 +14,9 @@ export const revalidate = 0
1314
*
1415
* Writes the version's bytes as a new version, so the revert can itself be reverted. Reverting to
1516
* the current version is a no-op that reports `reverted: false`.
17+
*
18+
* A revert invalidates the revision the caller guarded it with, so the response carries the one
19+
* naming the content the file now holds.
1620
*/
1721
export const POST = defineV2JsonRoute({
1822
contract: v2RevertFileVersionContract,
@@ -30,6 +34,13 @@ export const POST = defineV2JsonRoute({
3034
useCase: revertWorkspaceFileVersion,
3135
present: async ({ file, version, reverted }) => {
3236
const [v2File, v2Version] = await Promise.all([toV2File(file), toV2FileVersion(version)])
33-
return { data: { reverted, file: v2File, version: v2Version } }
37+
return {
38+
data: {
39+
reverted,
40+
file: v2File,
41+
version: v2Version,
42+
...workspaceFileRevisionField(file),
43+
},
44+
}
3445
},
3546
})
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import { describe, expect, it } from 'vitest'
2+
import { mergePresenceRoster } from '@/app/workspace/providers/socket-presence-merge'
3+
import type { PresenceUser } from '@/stores/presence/types'
4+
5+
function peer(overrides: Partial<PresenceUser> = {}): PresenceUser {
6+
return {
7+
socketId: 'socket-1',
8+
userId: 'user-1',
9+
userName: 'Ada',
10+
...overrides,
11+
}
12+
}
13+
14+
describe('mergePresenceRoster', () => {
15+
it('clears the pointer when the roster carries an explicit null cursor', () => {
16+
const previous = [peer({ cursor: { x: 10, y: 20 } })]
17+
18+
const merged = mergePresenceRoster(previous, [peer({ cursor: null })])
19+
20+
expect(merged[0].cursor).toBeNull()
21+
})
22+
23+
it('keeps the known pointer when the roster omits the cursor', () => {
24+
const previous = [peer({ cursor: { x: 10, y: 20 } })]
25+
26+
const merged = mergePresenceRoster(previous, [peer()])
27+
28+
expect(merged[0].cursor).toEqual({ x: 10, y: 20 })
29+
})
30+
31+
it('keeps the known selection when the roster omits it', () => {
32+
const previous = [peer({ selection: { type: 'block', id: 'block-1' } })]
33+
34+
const merged = mergePresenceRoster(previous, [peer()])
35+
36+
expect(merged[0].selection).toEqual({ type: 'block', id: 'block-1' })
37+
})
38+
39+
it('applies a cleared selection, which the wire spells as type none', () => {
40+
const previous = [peer({ selection: { type: 'block', id: 'block-1' } })]
41+
42+
const merged = mergePresenceRoster(previous, [peer({ selection: { type: 'none' } })])
43+
44+
expect(merged[0].selection).toEqual({ type: 'none' })
45+
})
46+
47+
it('passes through a peer it has no previous presence for', () => {
48+
const joining = peer({ socketId: 'socket-2', userId: 'user-2', cursor: { x: 1, y: 2 } })
49+
50+
expect(mergePresenceRoster([], [joining])).toEqual([joining])
51+
})
52+
53+
it('drops peers the roster no longer lists', () => {
54+
const previous = [peer(), peer({ socketId: 'socket-2', userId: 'user-2' })]
55+
56+
const merged = mergePresenceRoster(previous, [peer()])
57+
58+
expect(merged.map((user) => user.socketId)).toEqual(['socket-1'])
59+
})
60+
})
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
import type { PresenceUser } from '@/stores/presence/types'
2+
3+
/**
4+
* Folds a `presence-update` roster over the presence already held for each socket.
5+
*
6+
* The server rebuilds a socket's presence record from scratch when it joins a room, so a re-join
7+
* of the same workflow broadcasts a roster whose `cursor` and `selection` are simply absent. The
8+
* fields are carried over rather than blanked, which is what keeps a peer's pointer from
9+
* flickering on every re-join.
10+
*
11+
* A `null` `cursor` is therefore not the same as an absent one: it is a pointer the peer
12+
* explicitly cleared on leaving the canvas. Coalescing the two with `??` would resurrect a stale
13+
* pointer whenever the clearing `cursor-update` was missed — dropped by the visibility gate
14+
* during a join, or by a rejoin that never refreshed the roster. `selection` needs no such
15+
* split: a cleared selection is `{ type: 'none' }`, and the wire type admits no `null`.
16+
*/
17+
export function mergePresenceRoster(
18+
previous: PresenceUser[],
19+
incoming: PresenceUser[]
20+
): PresenceUser[] {
21+
const previousBySocketId = new Map(previous.map((user) => [user.socketId, user]))
22+
23+
return incoming.map((user) => {
24+
const existing = previousBySocketId.get(user.socketId)
25+
if (!existing) return user
26+
return {
27+
...user,
28+
cursor: user.cursor === undefined ? existing.cursor : user.cursor,
29+
selection: user.selection ?? existing.selection,
30+
}
31+
})
32+
}

0 commit comments

Comments
 (0)