Skip to content

Commit 7ed0655

Browse files
authored
fix(files): keep file search and compiled-doc reads off write locks (#9023)
* fix(files): keep file search and compiled-doc reads off write locks - Copilot search delivery no longer takes the Project mutex, the org-wide permission-group lock, or FOR UPDATE on permission rows; the transaction only reads, and admission runs after commit. - A matched file that changed between search and delivery gets unknown provenance (preview withheld) instead of failing the whole search; a file moved out of the folder scope is dropped; owned-input checks run only for current sources. - Serving a compiled doc skips the locked search requeue when the current revision is already built from the artifact, bounds the locked path with a 1s lock_timeout, and never fails a read whose bytes were fetched. - Index coverage derives pending files as the remainder so the dependency check runs once per ready file. - Document reference scanning skips sources with no reference marker and loads the TypeScript compiler on first parse; unparseable doc source is indexed as source text again. * fix(files): validate search delivery against one snapshot and count returned hits Run the Copilot delivery validation at repeatable read so the folder scope and the file rows come from one snapshot, and report count as the hits actually returned after out-of-scope hits are dropped. * refactor(files): move rendered-artifact search requeue into artifact-ready Route the bounded requeue through configureFileSearchTransaction so it carries the same statement and transaction guards as every other file search transaction, keep the lock-free pre-check private, drop the redundant require cache, and use line comments inside function bodies.
1 parent e13228a commit 7ed0655

9 files changed

Lines changed: 214 additions & 117 deletions

File tree

‎apps/sim/lib/uploads/documents/references.ts‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import ts from '@typescript/typescript6'
1+
import type ts from '@typescript/typescript6'
22
import { DocCompileUserError } from '@/lib/uploads/documents/compile-error'
33

44
export type DocumentSourceLanguage = 'javascript' | 'python'
@@ -16,17 +16,25 @@ interface SourceToken {
1616
}
1717

1818
const FILE_HELPERS = new Set(['getFileBase64', 'addImage', 'drawImage', 'input_path'])
19+
const INPUT_PATH_PREFIX = '/home/user/inputs/'
20+
/**
21+
* Every reference is a token whose raw source text is a helper name or starts with the input
22+
* path, so a source containing none of these substrings has no references and is never parsed.
23+
*/
24+
const REFERENCE_MARKERS = [...FILE_HELPERS, INPUT_PATH_PREFIX]
1925

2026
function* javascriptTokens(source: string): Generator<SourceToken> {
27+
// The compiler is large and most sources carry no reference, so it loads on first parse.
28+
const typescript = require('@typescript/typescript6') as typeof ts
2129
const fileName = 'document.js'
22-
const sourceFile = ts.createSourceFile(
30+
const sourceFile = typescript.createSourceFile(
2331
fileName,
2432
source,
25-
ts.ScriptTarget.Latest,
33+
typescript.ScriptTarget.Latest,
2634
true,
27-
ts.ScriptKind.JS
35+
typescript.ScriptKind.JS
2836
)
29-
const program = ts.createProgram(
37+
const program = typescript.createProgram(
3038
[fileName],
3139
{ allowJs: true, noLib: true, noResolve: true },
3240
{
@@ -44,14 +52,16 @@ function* javascriptTokens(source: string): Generator<SourceToken> {
4452
)
4553
const diagnostic = program.getSyntacticDiagnostics(sourceFile)[0]
4654
if (diagnostic) {
47-
throw new DocCompileUserError(ts.flattenDiagnosticMessageText(diagnostic.messageText, '\n'))
55+
throw new DocCompileUserError(
56+
typescript.flattenDiagnosticMessageText(diagnostic.messageText, '\n')
57+
)
4858
}
4959

5060
const pending: ts.Node[] = [sourceFile]
5161
while (pending.length > 0) {
5262
const node = pending.pop()
5363
if (!node) break
54-
if (ts.isStringLiteralLike(node)) {
64+
if (typescript.isStringLiteralLike(node)) {
5565
const start = node.getStart(sourceFile) + 1
5666
yield { kind: 'string', value: source.slice(start, node.end - 1), start }
5767
continue
@@ -62,7 +72,7 @@ function* javascriptTokens(source: string): Generator<SourceToken> {
6272
} else {
6373
const start = node.getStart(sourceFile)
6474
yield {
65-
kind: ts.isIdentifier(node) ? 'word' : 'punctuation',
75+
kind: typescript.isIdentifier(node) ? 'word' : 'punctuation',
6676
value: source.slice(start, node.end),
6777
start,
6878
}
@@ -165,6 +175,7 @@ export function* iterateDocumentFileReferences(
165175
source: string,
166176
language: DocumentSourceLanguage
167177
): Generator<DocumentFileReference> {
178+
if (!REFERENCE_MARKERS.some((marker) => source.includes(marker))) return
168179
const previous: SourceToken[] = []
169180
for (const token of language === 'javascript' ? javascriptTokens(source) : pythonTokens(source)) {
170181
if (token.kind === 'string') {
@@ -185,7 +196,7 @@ export function* iterateDocumentFileReferences(
185196
? token.value
186197
: undefined)
187198
if (fileId) {
188-
const start = token.start + (path ? '/home/user/inputs/'.length : 0)
199+
const start = token.start + (path ? INPUT_PATH_PREFIX.length : 0)
189200
yield { fileId, start, end: start + fileId.length }
190201
}
191202
}

‎apps/sim/lib/workspace-files/application/fetch-servable-workspace-file-buffer.ts‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
import type { Principal } from '@sim/auth/principal'
2-
import { db } from '@sim/db'
32
import { createLogger } from '@sim/logger'
3+
import { getErrorMessage } from '@sim/utils/errors'
44
import { generateRequestId } from '@/lib/core/utils/request'
55
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
66
import {
77
downloadServableFileFromStorage,
88
type ServableFile,
99
} from '@/lib/uploads/utils/file-utils.server'
10-
import { markFileSearchArtifactReadyInTx } from '@/lib/workspace-files/search/artifact-ready'
10+
import { requeueFileSearchForRenderedArtifact } from '@/lib/workspace-files/search/artifact-ready'
1111

1212
const logger = createLogger('FetchServableWorkspaceFileBuffer')
1313

@@ -45,8 +45,8 @@ export async function fetchAuthorizedServableWorkspaceFileBuffer(
4545
(file) => file.contentUpdatedAt && ['workspace', 'test', 'changelog'].includes(file.context)
4646
) !== false
4747
) {
48-
await db.transaction((tx) =>
49-
markFileSearchArtifactReadyInTx(tx, {
48+
try {
49+
await requeueFileSearchForRenderedArtifact({
5050
owner: { entityType: 'workspace', entityId: fileRecord.workspaceId },
5151
file: {
5252
fileId: fileRecord.id,
@@ -64,7 +64,12 @@ export async function fetchAuthorizedServableWorkspaceFileBuffer(
6464
}),
6565
artifactKey,
6666
})
67-
)
67+
} catch (error) {
68+
logger.warn('Could not requeue file search for a rendered artifact', {
69+
fileId: fileRecord.id,
70+
error: getErrorMessage(error),
71+
})
72+
}
6873
}
6974
return result
7075
}

‎apps/sim/lib/workspace-files/application/search-workspace-file-content.test.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import { createSessionPrincipal } from '@sim/testing/factories/principal.factory'
22
import { dbChainMockFns, queueTableRows, resetDbChainMock } from '@sim/testing/mocks/database.mock'
3-
import { projectMembershipMock } from '@sim/testing/mocks/project-membership.mock'
43
import { schemaMock } from '@sim/testing/mocks/schema.mock'
54
import { workspaceAuthzMock, workspaceAuthzMockFns } from '@sim/testing/mocks/workspace-authz.mock'
65
import {
@@ -19,7 +18,6 @@ const hoisted = vi.hoisted(() => ({
1918
}))
2019
vi.mock('@sim/platform-authz/workspace', () => workspaceAuthzMock)
2120
vi.mock('@/lib/uploads/contexts/workspace', () => workspaceUploadsMock)
22-
vi.mock('@/lib/projects/membership', () => projectMembershipMock)
2321
vi.mock('@/lib/workspace-files/search/repository', async (importOriginal) => ({
2422
...(await importOriginal<typeof import('@/lib/workspace-files/search/repository')>()),
2523
searchFileIndex: hoisted.search,

‎apps/sim/lib/workspace-files/application/search-workspace-file-content.ts‎

Lines changed: 46 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@ import { getPostgresErrorCode } from '@sim/utils/errors'
44
import { and, eq, isNull } from 'drizzle-orm'
55
import { admitEach, mustClassifyContent } from '@/lib/core/application'
66
import { OrchestrationError } from '@/lib/core/orchestration/types'
7-
import { acquirePermissionGroupOrgLock } from '@/lib/permission-groups/locks'
8-
import { lockWorkspaceProject } from '@/lib/projects/membership'
97
import { listFileFolders, loadActiveWorkspaceContext } from '@/lib/uploads/contexts/workspace'
108
import { authorizeWorkspaceFileAccess } from '@/lib/workspace-files/application/authorization'
119
import { defineAuthorizedWorkspaceFileUseCase } from '@/lib/workspace-files/application/authorized-workspace-file-use-case'
@@ -102,56 +100,66 @@ export const searchWorkspaceFileContent = defineAuthorizedWorkspaceFileUseCase({
102100
})
103101
return toWorkspaceFileSearchResult(result)
104102
}
105-
const provenance = await db.transaction(async (tx) => {
106-
await lockWorkspaceProject(tx, context.workspaceId)
107-
const [current] = await tx
108-
.select({
109-
workspaceId: workspace.id,
110-
billedAccountUserId: workspace.billedAccountUserId,
111-
workspaceOrganizationId: workspace.organizationId,
112-
allowPersonalApiKeys: workspace.allowPersonalApiKeys,
103+
// One repeatable-read snapshot keeps the folder scope and file rows consistent with each other.
104+
// Admission runs after commit, so row or advisory locks would fence nothing.
105+
const delivery = await db.transaction(
106+
async (tx) => {
107+
const [current] = await tx
108+
.select({
109+
workspaceId: workspace.id,
110+
billedAccountUserId: workspace.billedAccountUserId,
111+
workspaceOrganizationId: workspace.organizationId,
112+
allowPersonalApiKeys: workspace.allowPersonalApiKeys,
113+
})
114+
.from(workspace)
115+
.where(and(eq(workspace.id, context.workspaceId), isNull(workspace.archivedAt)))
116+
if (!current) throw new OrchestrationError('not_found', 'Workspace not found')
117+
await authorizeWorkspaceFileAccess(principal, fileOperations.searchContent, current, {
118+
executor: tx,
113119
})
114-
.from(workspace)
115-
.where(and(eq(workspace.id, context.workspaceId), isNull(workspace.archivedAt)))
116-
.for('share')
117-
if (!current) throw new OrchestrationError('not_found', 'Workspace not found')
118-
if (current.workspaceOrganizationId)
119-
await acquirePermissionGroupOrgLock(tx, current.workspaceOrganizationId)
120-
await authorizeWorkspaceFileAccess(principal, fileOperations.searchContent, current, {
121-
executor: tx,
122-
forUpdate: true,
123-
})
124-
const folders = input.folderPaths === undefined ? [] : await listFileFolders(owner, {}, tx)
125-
const delivery = await loadFileSearchDelivery(tx, {
126-
owner,
127-
prepared: result,
128-
scope: resolveFileSearchFolderScope(folders, input),
129-
signal,
130-
classify: classified,
131-
})
132-
for (const dependency of delivery.inputs)
133-
await assertOwnedFileAccess(principal, { ...current, ...dependency }, tx)
134-
return delivery.provenance
135-
})
136-
const admitted = await admitEach(principal, result.sources, async (sources) =>
120+
const folders =
121+
input.folderPaths === undefined ? [] : await listFileFolders(owner, {}, tx)
122+
const loaded = await loadFileSearchDelivery(tx, {
123+
owner,
124+
prepared: result,
125+
scope: resolveFileSearchFolderScope(folders, input),
126+
signal,
127+
classify: classified,
128+
})
129+
for (const dependency of loaded.inputs)
130+
await assertOwnedFileAccess(principal, { ...current, ...dependency }, tx)
131+
return loaded
132+
},
133+
{ isolationLevel: 'repeatable read' }
134+
)
135+
const inScope = result.sources.filter(
136+
(source) => !delivery.outOfScope.has(source.identity.fileId)
137+
)
138+
const admitted = await admitEach(principal, inScope, async (sources) =>
137139
sources.map((source) => ({
138140
kind: 'workspace-file' as const,
139141
id: source.identity.fileId,
140-
provenance: provenance.get(source.identity.fileId) ?? { status: 'unknown' as const },
142+
provenance: delivery.provenance.get(source.identity.fileId) ?? {
143+
status: 'unknown' as const,
144+
},
141145
view: 'excerpt' as const,
142146
}))
143147
)
144148
const admittedIds = new Set(admitted.map((source) => source.identity.fileId))
145-
return toWorkspaceFileSearchResult({
146-
...result,
147-
results: result.results.map((hit) =>
149+
const results = result.results
150+
.filter((hit) => !delivery.outOfScope.has(hit.fileId))
151+
.map((hit) =>
148152
admittedIds.has(hit.fileId)
149153
? hit
150154
: {
151155
...hit,
152156
text: '[Preview withheld: read or grep this file to inspect protected text.]',
153157
}
154-
),
158+
)
159+
return toWorkspaceFileSearchResult({
160+
...result,
161+
results,
162+
count: results.length,
155163
sources: admitted,
156164
})
157165
} catch (error) {

‎apps/sim/lib/workspace-files/search/artifact-ready.ts‎

Lines changed: 73 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,21 @@
1+
import { db } from '@sim/db'
12
import {
23
fileSearchDispatchQueue,
34
workspaceFileSearchBuild,
45
workspaceFileSearchRevision,
56
} from '@sim/db/schema'
6-
import { and, eq, sql } from 'drizzle-orm'
7+
import { and, eq, or, sql } from 'drizzle-orm'
78
import type { DbTransaction } from '@/lib/db/types'
89
import type { EditableFileOwner } from '@/lib/workspace-files/ownership'
10+
import { FILE_SEARCH_ARTIFACT_READY_LOCK_TIMEOUT_MS } from '@/lib/workspace-files/search/constants'
911
import { lockFileSearchInputs } from '@/lib/workspace-files/search/index-state'
1012
import {
1113
fileSearchOwnerCondition,
1214
fileSearchOwnerFields,
1315
resolveFileSearchOwner,
1416
} from '@/lib/workspace-files/search/scope'
1517
import type { FileSearchDependencyIdentity } from '@/lib/workspace-files/search/source'
18+
import { configureFileSearchTransaction } from '@/lib/workspace-files/search/transaction'
1619

1720
/** A renderer's private receipt; surfaces cannot supply it through a file contract. */
1821
interface FileSearchArtifactCompletion {
@@ -22,11 +25,8 @@ interface FileSearchArtifactCompletion {
2225
artifactKey: string
2326
}
2427

25-
/** Requeues only current stored inputs and fences workers that read before the artifact existed. */
26-
export async function markFileSearchArtifactReadyInTx(
27-
tx: DbTransaction,
28-
input: FileSearchArtifactCompletion
29-
): Promise<boolean> {
28+
/** Pure check that the artifact key was minted for this file owner; throws otherwise. */
29+
function assertFileSearchArtifactReceipt(input: FileSearchArtifactCompletion) {
3030
const owner = resolveFileSearchOwner(input)
3131
const prefix =
3232
owner.entityType === 'workspace'
@@ -37,17 +37,79 @@ export async function markFileSearchArtifactReadyInTx(
3737
!/^[a-f0-9]{64}\.(docx|pptx|pdf|xlsx)$/.test(input.artifactKey.slice(prefix.length))
3838
)
3939
throw new Error('Artifact receipt does not belong to this file owner')
40+
return owner
41+
}
42+
43+
function artifactRevisionCondition(input: FileSearchArtifactCompletion, owner: EditableFileOwner) {
44+
return and(
45+
eq(workspaceFileSearchRevision.fileId, input.file.fileId),
46+
fileSearchOwnerCondition(workspaceFileSearchRevision, owner),
47+
eq(workspaceFileSearchRevision.sourceContentUpdatedAt, input.file.contentUpdatedAt)
48+
)
49+
}
50+
51+
/**
52+
* Lock-free form of {@link markFileSearchArtifactReadyInTx}'s no-op test: the file's current
53+
* revision is already built from this artifact, ready or still under a live build lease.
54+
*/
55+
async function isFileSearchArtifactCurrent(
56+
input: FileSearchArtifactCompletion,
57+
owner: EditableFileOwner
58+
): Promise<boolean> {
59+
const [current] = await db
60+
.select({ fileId: workspaceFileSearchRevision.fileId })
61+
.from(workspaceFileSearchRevision)
62+
.innerJoin(
63+
workspaceFileSearchBuild,
64+
eq(workspaceFileSearchBuild.id, workspaceFileSearchRevision.buildId)
65+
)
66+
.where(
67+
and(
68+
artifactRevisionCondition(input, owner),
69+
eq(workspaceFileSearchBuild.artifactKey, input.artifactKey),
70+
or(
71+
eq(workspaceFileSearchRevision.status, 'ready'),
72+
and(
73+
eq(workspaceFileSearchRevision.status, 'pending'),
74+
sql`${workspaceFileSearchBuild.expiresAt} > clock_timestamp()`
75+
)
76+
)
77+
)
78+
)
79+
.limit(1)
80+
return current !== undefined
81+
}
82+
83+
/**
84+
* Requeues search for a freshly rendered artifact unless its revision already uses it. The common
85+
* already-current case takes no locks; the locked requeue is bounded by a short lock timeout.
86+
*/
87+
export async function requeueFileSearchForRenderedArtifact(
88+
input: FileSearchArtifactCompletion
89+
): Promise<void> {
90+
const owner = assertFileSearchArtifactReceipt(input)
91+
if (await isFileSearchArtifactCurrent(input, owner)) return
92+
await db.transaction(async (tx) => {
93+
await configureFileSearchTransaction(tx, {
94+
lockTimeout: FILE_SEARCH_ARTIFACT_READY_LOCK_TIMEOUT_MS,
95+
})
96+
await markFileSearchArtifactReadyInTx(tx, input)
97+
})
98+
}
99+
100+
/** Requeues only current stored inputs and fences workers that read before the artifact existed. */
101+
export async function markFileSearchArtifactReadyInTx(
102+
tx: DbTransaction,
103+
input: FileSearchArtifactCompletion
104+
): Promise<boolean> {
105+
const owner = assertFileSearchArtifactReceipt(input)
40106
const revision = {
41107
owner,
42108
fileId: input.file.fileId,
43109
sourceContentUpdatedAt: input.file.contentUpdatedAt,
44110
}
45111
if (!(await lockFileSearchInputs(tx, revision, input.dependencies, input.file.key))) return false
46-
const predicate = and(
47-
eq(workspaceFileSearchRevision.fileId, revision.fileId),
48-
fileSearchOwnerCondition(workspaceFileSearchRevision, owner),
49-
eq(workspaceFileSearchRevision.sourceContentUpdatedAt, revision.sourceContentUpdatedAt)
50-
)
112+
const predicate = artifactRevisionCondition(input, owner)
51113
const [state] = await tx.select().from(workspaceFileSearchRevision).where(predicate).limit(1)
52114
if (state?.buildId) {
53115
const [build] = await tx

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ export const FILE_SEARCH_PATTERN_MAX_DEPTH = 20
1414
/** Total search deadline, including patterns whose literals cannot provide a selective trigram probe. */
1515
export const FILE_SEARCH_STATEMENT_TIMEOUT_MS = 10 * 1000
1616
export const FILE_SEARCH_LOCK_TIMEOUT_MS = 5 * 1000
17+
/** A rendered file's bytes are already fetched, so a contended requeue gives up rather than stall the read. */
18+
export const FILE_SEARCH_ARTIFACT_READY_LOCK_TIMEOUT_MS = 1000
1719
export const FILE_SEARCH_DEFAULT_MAX_RESULTS = 50
1820
export const FILE_SEARCH_MAX_RESULTS = 200
1921

0 commit comments

Comments
 (0)