Skip to content

Commit a6cec88

Browse files
committed
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.
1 parent 081dd7c commit a6cec88

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": "d2ZfVjFTdEdYUjh6NWpkSGk2Qm15VDkxOjIwMjYtMDEtMTZUMDk6MTI6MDAuMDAwWg"
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)