Skip to content

Commit 5461c76

Browse files
authored
fix(file-search): send legacy workspace index payloads, isolate realtime test DB, best-effort post-upload Project check (#9041)
* fix(file-search): send legacy workspace index payloads, isolate realtime test DB, best-effort post-upload Project check - Workspace index jobs use the { workspaceId } wire shape so a rolled-back worker can decode them; Project jobs keep { owner } - Realtime integration setup clears inherited role-specific DB URLs and runs as the realtime role on the test database, via a shared clearInheritedDatabaseUrls helper - After a successful local upload write, a failed retired-Project check logs and returns 204 instead of 500 * fix(uploads): name the expired-session sweep as the post-write cleanup backstop
1 parent bbfbf4e commit 5461c76

10 files changed

Lines changed: 121 additions & 34 deletions

File tree

‎apps/realtime/vitest.integration.setup.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
1-
import { readTestDatabaseUrl, readTestRedisUrl } from '@sim/db/testing/test-infrastructure'
1+
import {
2+
clearInheritedDatabaseUrls,
3+
readTestDatabaseUrl,
4+
readTestRedisUrl,
5+
} from '@sim/db/testing/test-infrastructure'
26

37
const redisUrl = readTestRedisUrl()
48
if (!redisUrl) throw new Error('Realtime integration tests require disposable TEST_REDIS_URL')
59

10+
clearInheritedDatabaseUrls()
611
Object.assign(process.env, {
712
NODE_ENV: 'test',
13+
SIM_DB_ROLE: 'realtime',
814
DATABASE_URL: readTestDatabaseUrl(),
915
REDIS_URL: redisUrl,
1016
BETTER_AUTH_URL: 'http://localhost:3000',

‎apps/sim/app/api/v2/uploads/[uploadId]/parts/[partNumber]/route.test.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { dbChainMockFns } from '@sim/testing/mocks/database.mock'
12
import { uploadSessionMock, uploadSessionMockFns } from '@sim/testing/mocks/upload-session.mock'
23
import { NextRequest } from 'next/server'
34
import { beforeEach, describe, expect, it, vi } from 'vitest'
@@ -18,6 +19,7 @@ vi.mock('@/lib/uploads/upload-session/provider', () => ({
1819
vi.mock('@/lib/uploads/upload-session/service', () => uploadSessionMock)
1920

2021
import { OrchestrationError } from '@/lib/core/orchestration/types'
22+
import { PROJECT_FILE_UPLOAD_BINDING_KEY } from '@/lib/uploads/upload-session/types'
2123
import { PUT } from '@/app/api/v2/uploads/[uploadId]/parts/[partNumber]/route'
2224

2325
const mockExpectedUploadPartSize = uploadSessionMockFns.mockExpectedUploadPartSize
@@ -54,6 +56,24 @@ describe('PUT /api/v2/uploads/[uploadId]/parts/[partNumber]', () => {
5456
})
5557
})
5658

59+
it('answers 204 when the retired-Project check fails after the part is written', async () => {
60+
mockVerifyUploadSessionToken.mockReturnValue({
61+
...SESSION,
62+
purpose: 'project_file',
63+
storageProvider: 'local',
64+
providerUploadId: null,
65+
finalKey: 'project/project-1/file.bin',
66+
metadata: { [PROJECT_FILE_UPLOAD_BINDING_KEY]: { entityId: 'project-1' } },
67+
})
68+
dbChainMockFns.transaction.mockRejectedValueOnce(new Error('database unavailable'))
69+
70+
const response = await request()
71+
72+
expect(response.status).toBe(204)
73+
expect(mockWriteLocalMultipartPart).toHaveBeenCalledOnce()
74+
expect(dbChainMockFns.transaction).toHaveBeenCalledOnce()
75+
})
76+
5777
it('maps a streamed-size failure to 400', async () => {
5878
mockWriteLocalMultipartPart.mockRejectedValue(
5979
new MockLocalUploadBodyError('Part 1 has 2 bytes; expected 3')

‎apps/sim/app/api/v2/uploads/[uploadId]/parts/[partNumber]/route.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,17 @@ export const PUT = withRouteHandler(
105105
}
106106
throw error
107107
}
108-
if (await queueRetiredProjectUploadCleanup(session))
109-
return v2Error('CONFLICT', 'The Project no longer exists')
108+
try {
109+
if (await queueRetiredProjectUploadCleanup(session))
110+
return v2Error('CONFLICT', 'The Project no longer exists')
111+
} catch (error) {
112+
// The object is already written. The expired-session sweep cleans it up, as will completion's
113+
// Project re-authorization once Project uploads can complete.
114+
logger.warn('Could not check for a retired Project after upload', {
115+
uploadId: session.id,
116+
error,
117+
})
118+
}
110119
return new NextResponse(null, { status: 204 })
111120
},
112121
{

‎apps/sim/app/api/v2/uploads/[uploadId]/route.test.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { dbChainMockFns } from '@sim/testing/mocks/database.mock'
12
import { uploadSessionMock, uploadSessionMockFns } from '@sim/testing/mocks/upload-session.mock'
23
import { NextRequest } from 'next/server'
34
import { beforeEach, describe, expect, it, vi } from 'vitest'
@@ -17,6 +18,7 @@ vi.mock('@/lib/uploads/upload-session/provider', () => ({
1718

1819
vi.mock('@/lib/uploads/upload-session/service', () => uploadSessionMock)
1920

21+
import { PROJECT_FILE_UPLOAD_BINDING_KEY } from '@/lib/uploads/upload-session/types'
2022
import { PUT } from '@/app/api/v2/uploads/[uploadId]/route'
2123

2224
const { mockGetOwnedUploadSession, mockUploadSessionObjectMetadata: mockMetadata } =
@@ -117,6 +119,23 @@ describe('PUT /api/v2/uploads/[uploadId]', () => {
117119
expect(mockWriteLocalPut).not.toHaveBeenCalled()
118120
})
119121

122+
it('answers 204 when the retired-Project check fails after the object is written', async () => {
123+
mockGetOwnedUploadSession.mockReturnValue({
124+
...SESSION,
125+
purpose: 'project_file',
126+
storageContext: 'project',
127+
finalKey: 'project/project-1/file.bin',
128+
metadata: { [PROJECT_FILE_UPLOAD_BINDING_KEY]: { entityId: 'project-1' } },
129+
})
130+
dbChainMockFns.transaction.mockRejectedValueOnce(new Error('database unavailable'))
131+
132+
const response = await request()
133+
134+
expect(response.status).toBe(204)
135+
expect(mockWriteLocalPut).toHaveBeenCalledOnce()
136+
expect(dbChainMockFns.transaction).toHaveBeenCalledOnce()
137+
})
138+
120139
it('maps exact-size streaming failures to a caller error', async () => {
121140
mockWriteLocalPut.mockRejectedValue(new MockLocalUploadBodyError('Upload exceeds 3 bytes'))
122141

‎apps/sim/app/api/v2/uploads/[uploadId]/route.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,17 @@ export const PUT = withRouteHandler(
9595
}
9696
throw error
9797
}
98-
if (await queueRetiredProjectUploadCleanup(session))
99-
return v2Error('CONFLICT', 'The Project no longer exists')
98+
try {
99+
if (await queueRetiredProjectUploadCleanup(session))
100+
return v2Error('CONFLICT', 'The Project no longer exists')
101+
} catch (error) {
102+
// The object is already written. The expired-session sweep cleans it up, as will completion's
103+
// Project re-authorization once Project uploads can complete.
104+
logger.warn('Could not check for a retired Project after upload', {
105+
uploadId: session.id,
106+
error,
107+
})
108+
}
100109
return new NextResponse(null, { status: 204 })
101110
},
102111
{

‎apps/sim/lib/workspace-files/search/dispatcher.integration.ts‎

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,9 @@ describe('workspace file search dispatch PostgreSQL deadlines', () => {
261261

262262
const first = await prepareWorkspaceFileSearchDispatch()
263263
const second = await prepareWorkspaceFileSearchDispatch()
264-
expect([...first.payloads, ...second.payloads].map((payload) => payload.owner)).toEqual([
265-
{ entityType: 'workspace', entityId: 'later-workspace' },
266-
])
264+
const claimed = [...first.payloads, ...second.payloads]
265+
expect(claimed.map((payload) => payload.workspaceId)).toEqual(['later-workspace'])
266+
expect(claimed[0]).not.toHaveProperty('owner')
267267
const [queued] =
268268
await connection`SELECT count(*)::int AS count FROM file_search_dispatch_queue
269269
WHERE entity_type = 'project'`
@@ -272,9 +272,10 @@ describe('workspace file search dispatch PostgreSQL deadlines', () => {
272272
await holder`COMMIT`
273273
const resumed = await prepareWorkspaceFileSearchDispatch()
274274
expect(resumed.payloads.length).toBeGreaterThan(0)
275-
expect(resumed.payloads.every((payload) => payload.owner?.entityType === 'project')).toBe(
276-
true
277-
)
275+
for (const payload of resumed.payloads) {
276+
expect(payload.owner?.entityType).toBe('project')
277+
expect(payload).not.toHaveProperty('workspaceId')
278+
}
278279
} finally {
279280
await holder`ROLLBACK`
280281
holder.release()
@@ -290,17 +291,16 @@ describe('workspace file search dispatch PostgreSQL deadlines', () => {
290291
await holder`SELECT pg_advisory_xact_lock(hashtextextended('project:busy-project', 0))`
291292
const result = await prepareWorkspaceFileSearchDispatch()
292293
expect(result.payloads).toHaveLength(1)
293-
expect(result.payloads[0].owner).toEqual({
294-
entityType: 'workspace',
295-
entityId: 'free-workspace',
296-
})
294+
expect(result.payloads[0].workspaceId).toBe('free-workspace')
295+
expect(result.payloads[0]).not.toHaveProperty('owner')
297296
const [waiting] = await connection`SELECT count(*)::int AS count
298297
FROM file_search_dispatch_queue WHERE entity_type = 'project' AND entity_id = 'busy-project'`
299298
expect(waiting.count).toBe(1)
300299
await holder`COMMIT`
301300
const resumed = await prepareWorkspaceFileSearchDispatch()
302301
expect(resumed.payloads).toHaveLength(1)
303302
expect(resumed.payloads[0].owner).toEqual({ entityType: 'project', entityId: 'busy-project' })
303+
expect(resumed.payloads[0]).not.toHaveProperty('workspaceId')
304304
} finally {
305305
await holder`ROLLBACK`
306306
holder.release()
@@ -311,13 +311,16 @@ describe('workspace file search dispatch PostgreSQL deadlines', () => {
311311
await seedQueue('shared-id', 2, 1)
312312
await seedQueue('shared-id', 2, 0, 'project')
313313
const result = await prepareWorkspaceFileSearchDispatch()
314-
expect(
315-
result.payloads.filter((payload) => payload.owner?.entityType === 'workspace')
316-
).toHaveLength(1)
317-
expect(
318-
result.payloads.filter((payload) => payload.owner?.entityType === 'project')
319-
).toHaveLength(2)
320-
expect(result.payloads.every((payload) => payload.owner?.entityId === 'shared-id')).toBe(true)
314+
const scopes = result.payloads.map((payload) =>
315+
'workspaceId' in payload
316+
? `legacy:${payload.workspaceId}`
317+
: `${payload.owner?.entityType}:${payload.owner?.entityId}`
318+
)
319+
expect([...scopes].sort()).toEqual([
320+
'legacy:shared-id',
321+
'project:shared-id',
322+
'project:shared-id',
323+
])
321324
const [invalid] =
322325
await connection`SELECT count(*)::int AS count FROM workspace_file_search_revision
323326
WHERE entity_type = 'project' AND workspace_id IS NOT NULL`
@@ -775,7 +778,7 @@ describe('workspace file search dispatch PostgreSQL deadlines', () => {
775778
payload: {
776779
dispatchToken: expect.any(String),
777780
fileId,
778-
owner: { entityType: 'workspace', entityId: workspaceId },
781+
workspaceId,
779782
sourceContentUpdatedAt: '2026-09-16T00:00:00.000Z',
780783
},
781784
}),

‎apps/sim/lib/workspace-files/search/dispatcher.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ import {
6262
import { lockFileSearchOwners } from '@/lib/workspace-files/search/owner-policy'
6363
import {
6464
fileSearchOwnerFields,
65+
fileSearchOwnerScope,
6566
resolveFileSearchOwner,
6667
staleFileSearchDependencyBuilds,
6768
} from '@/lib/workspace-files/search/scope'
@@ -510,7 +511,7 @@ async function claimQueuedOwnerJobs(
510511
.delete(fileSearchDispatchQueue)
511512
.where(and(queuedOwnerFilter(owners), notExists(pendingRevision), ...keepsStaleRevision))
512513
return rows.map((row) => ({
513-
owner: rowOwner(row),
514+
...fileSearchOwnerScope(rowOwner(row)),
514515
fileId: row.fileId,
515516
sourceContentUpdatedAt: new Date(row.sourceContentUpdatedAt).toISOString(),
516517
dispatchToken: now.toISOString(),

‎apps/sim/lib/workspace-files/search/scope.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,14 @@ export function resolveFileSearchOwner(input: unknown): EditableFileOwner {
3535
throw new Error('Invalid file search owner')
3636
}
3737

38+
/**
39+
* Encodes an owner for an index job. Workspace jobs keep the legacy `{ workspaceId }` shape so a
40+
* rolled-back worker can still decode them; only Project jobs carry `owner`.
41+
*/
42+
export function fileSearchOwnerScope(owner: EditableFileOwner): FileSearchOwnerScope {
43+
return owner.entityType === 'workspace' ? { workspaceId: owner.entityId } : { owner }
44+
}
45+
3846
/** Writes canonical owner columns and the legacy workspace key used by deployed workers. */
3947
export function fileSearchOwnerFields(owner: EditableFileOwner) {
4048
return {

‎apps/sim/vitest.integration.setup.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
import { readFileSync } from 'node:fs'
22
import { createServer } from 'node:http'
3-
import { readTestDatabaseUrl, readTestRedisUrl } from '@sim/db/testing/test-infrastructure'
3+
import {
4+
clearInheritedDatabaseUrls,
5+
readTestDatabaseUrl,
6+
readTestRedisUrl,
7+
} from '@sim/db/testing/test-infrastructure'
48
import { afterAll } from 'vitest'
59

610
/**
@@ -20,15 +24,7 @@ const environmentSource = readFileSync(new URL('./lib/core/config/env.ts', impor
2024
for (const entry of environmentSource.matchAll(/^\s+([A-Z][A-Z0-9_]*)\s*:/gm)) {
2125
delete process.env[entry[1]]
2226
}
23-
for (const key of Object.keys(process.env)) {
24-
if (
25-
key.startsWith('DATABASE_URL') ||
26-
key.startsWith('DATABASE_REPLICA_URL') ||
27-
key === 'MIGRATION_DATABASE_URL'
28-
) {
29-
delete process.env[key]
30-
}
31-
}
27+
clearInheritedDatabaseUrls()
3228
Object.assign(process.env, {
3329
NODE_ENV: 'test',
3430
DATABASE_URL: databaseUrl,

‎packages/db/testing/test-infrastructure.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,22 @@ export function readTestDatabaseUrl(): string {
3838
return value
3939
}
4040

41+
/**
42+
* Deletes every inherited database URL. `@sim/db` prefers a role-specific `DATABASE_URL_<ROLE>`
43+
* over `DATABASE_URL`, so one left in the shell would bypass the test database.
44+
*/
45+
export function clearInheritedDatabaseUrls(): void {
46+
for (const key of Object.keys(process.env)) {
47+
if (
48+
key.startsWith('DATABASE_URL') ||
49+
key.startsWith('DATABASE_REPLICA_URL') ||
50+
key === 'MIGRATION_DATABASE_URL'
51+
) {
52+
delete process.env[key]
53+
}
54+
}
55+
}
56+
4157
/**
4258
* Returns the validated `TEST_REDIS_URL`, or undefined when a local run has no Redis. In CI a
4359
* missing value throws: every Redis-gated suite would otherwise pass by skipping.

0 commit comments

Comments
 (0)