Skip to content

Commit e1451e9

Browse files
authored
fix(project-files): requeue rendered-artifact search after commit and trim Project read transactions (#9063)
* fix(project-files): requeue rendered-artifact search after commit and trim Project read transactions - Project artifact reads no longer requeue search inside the read transaction, which held the Project and source row shared and then needed them exclusively; the requeue runs after commit with its owner lock bounded by the short artifact-ready lock timeout and is best effort - Artifact, inline, and CSV preview reads authorize from their own snapshots instead of a separate preflight; artifact inputs are snapshotted only when referenced and download with bounded concurrency after a declared-size budget check; the request signal reaches render and compile - Project inline delivery streams and caches content-addressed key requests like workspace inline - Project search admits, withholds previews, and reports provenance after its snapshot commits, classifies provenance only when a gate or delivery observer needs it, and lists folders directly - A Project page embedding another owner's image is a missing input instead of a 500 - Page limits are shared from page-document.server; knip and the CI isolate build are tightened * test(project-files): start requeue checks from no search revision
1 parent e111f4f commit e1451e9

13 files changed

Lines changed: 574 additions & 310 deletions

File tree

‎.github/workflows/checks.yml‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,14 @@ jobs:
243243
working-directory: packages/db
244244
run: bun run db:migrate
245245

246+
# JOBS=4 caps node-gyp parallelism as docker/app.Dockerfile does (laverdet/isolated-vm#428).
246247
- name: Build native isolate addon for Project file rendering
247248
if: matrix.group == 'project-files'
248249
working-directory: apps/sim
250+
env:
251+
JOBS: 4
249252
run: |
250-
addon_dir="$(node -p 'require("node:path").dirname(require.resolve("isolated-vm/package.json", { paths: ["./apps/sim"] }))')"
253+
addon_dir="$(node -p 'require("node:path").dirname(require.resolve("isolated-vm/package.json", { paths: ["."] }))')"
251254
cd "$addon_dir"
252255
bun run install
253256

‎apps/sim/app/api/projects/[id]/files/inline/route.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@ import {
55
internalRateLimits,
66
internalSessionAuth,
77
} from '@/lib/api/server/routes'
8-
import { presentProjectFileContent } from '@/lib/projects/files/api'
98
import { readProjectInlineFile } from '@/lib/projects/files/application'
9+
import { presentFileDelivery, workspaceFileCacheControl } from '@/lib/uploads/server/delivery'
1010

1111
export const GET = defineInternalBinaryRoute({
1212
contract: getInlineProjectFileContract,
@@ -21,5 +21,12 @@ export const GET = defineInternalBinaryRoute({
2121
referenceFileId: query.fileId,
2222
}),
2323
useCase: readProjectInlineFile,
24-
present: presentProjectFileContent,
24+
present: ({ file, stream, contentLength, contentAddressed }) =>
25+
presentFileDelivery({
26+
body: stream,
27+
filename: file.name,
28+
contentType: file.type,
29+
contentLength,
30+
cacheControl: workspaceFileCacheControl(contentAddressed),
31+
}),
2532
})

‎apps/sim/lib/projects/files/__integration__/content-rendered.integration.ts‎

Lines changed: 140 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { dirname, join } from 'node:path'
55
import type { ResourceDelegatedPrincipal } from '@sim/auth/principal'
66
import { db } from '@sim/db'
77
import {
8+
fileSearchDispatchQueue,
89
folder,
910
idempotencyKey,
1011
member,
@@ -14,6 +15,7 @@ import {
1415
subscription,
1516
user,
1617
workspace,
18+
workspaceFileSearchRevision,
1719
workspaceFiles,
1820
} from '@sim/db/schema'
1921
import { deleteWorkspaceFixture, insertWorkspaceFixture } from '@sim/db/testing/workspace-fixtures'
@@ -44,11 +46,13 @@ import {
4446
} from '@/lib/projects/files/application/previews'
4547
import { searchProjectFileContent } from '@/lib/projects/files/application/search'
4648
import { revertProjectFileVersion } from '@/lib/projects/files/application/versions'
49+
import { lockProjectShared } from '@/lib/projects/membership'
4750
import * as storageCleanup from '@/lib/uploads/contexts/workspace/file-storage-cleanup-outbox'
4851
import * as storage from '@/lib/uploads/core/storage-service'
4952
import { compileFileDocument } from '@/lib/uploads/documents/compile'
5053
import * as heic from '@/lib/uploads/server/heic'
5154
import { observeWorkspaceFileDelivery } from '@/lib/workspace-files/application/file-delivery-observer'
55+
import { FILE_SEARCH_ARTIFACT_READY_LOCK_TIMEOUT_MS } from '@/lib/workspace-files/search/constants'
5256
import * as searchDelivery from '@/lib/workspace-files/search/delivery'
5357
import * as searchRepository from '@/lib/workspace-files/search/repository'
5458

@@ -139,6 +143,19 @@ async function fixture() {
139143
return { ...ids, principal: createSessionPrincipal({ userId: editorId }) }
140144
}
141145

146+
/** File creation marks a revision pending, so a requeue check starts from no revision at all. */
147+
async function clearSearchRevision(fileId: string) {
148+
await db.delete(workspaceFileSearchRevision).where(eq(workspaceFileSearchRevision.fileId, fileId))
149+
}
150+
151+
async function searchRevision(fileId: string) {
152+
const [revision] = await db
153+
.select()
154+
.from(workspaceFileSearchRevision)
155+
.where(eq(workspaceFileSearchRevision.fileId, fileId))
156+
return revision
157+
}
158+
142159
function createInput(projectId: string, content = 'Project architecture') {
143160
return {
144161
projectId,
@@ -306,6 +323,7 @@ describe('Project delivery admission', () => {
306323
}
307324
vi.spyOn(searchRepository, 'searchFileIndex').mockResolvedValue(prepared)
308325
vi.spyOn(searchDelivery, 'loadFileSearchDelivery').mockResolvedValue({
326+
current: new Set(),
309327
provenance: new Map(),
310328
outOfScope: new Set(['moved']),
311329
inputs: [],
@@ -671,6 +689,125 @@ describe('Project rendered artifacts against PostgreSQL and private storage', ()
671689
}
672690
)
673691

692+
check(
693+
'a generated artifact read beside a concurrent Project reader completes and requeues search',
694+
async () => {
695+
const f = await fixture()
696+
const source = await createProjectFile.execute({
697+
principal: f.principal,
698+
input: {
699+
...createInput(f.projectId, 'const title = "concurrent"'),
700+
name: 'deck.pptx',
701+
contentType: 'text/x-pptxgenjs',
702+
},
703+
})
704+
const artifact = Buffer.from('PK\u0003\u0004document')
705+
vi.spyOn(sandboxTask, 'runSandboxTask').mockResolvedValue(artifact)
706+
let markHeld = () => {}
707+
const held = new Promise<void>((resolve) => {
708+
markHeld = resolve
709+
})
710+
let release = () => {}
711+
const released = new Promise<void>((resolve) => {
712+
release = resolve
713+
})
714+
const concurrentReader = db.transaction(async (tx) => {
715+
await lockProjectShared(tx, f.projectId)
716+
markHeld()
717+
await released
718+
})
719+
await held
720+
await clearSearchRevision(source.file.id)
721+
try {
722+
const rendered = await observeWorkspaceFileDelivery(
723+
async () => release(),
724+
() =>
725+
readProjectFileArtifact.execute({
726+
principal: f.principal,
727+
input: { projectId: f.projectId, fileId: source.file.id, maxBytes: 1_000_000 },
728+
})
729+
)
730+
expect(rendered.buffer).toEqual(artifact)
731+
} finally {
732+
release()
733+
await concurrentReader
734+
}
735+
const revision = await searchRevision(source.file.id)
736+
expect(revision?.status).toBe('pending')
737+
expect(revision?.sourceContentUpdatedAt).toEqual(source.file.contentUpdatedAt)
738+
}
739+
)
740+
741+
check(
742+
'a contended search requeue gives up within its short bound without failing the read',
743+
async () => {
744+
const f = await fixture()
745+
const source = await createProjectFile.execute({
746+
principal: f.principal,
747+
input: {
748+
...createInput(f.projectId, 'const title = "contended"'),
749+
name: 'deck.pptx',
750+
contentType: 'text/x-pptxgenjs',
751+
},
752+
})
753+
const artifact = Buffer.from('PK\u0003\u0004document')
754+
vi.spyOn(sandboxTask, 'runSandboxTask').mockResolvedValue(artifact)
755+
let markHeld = () => {}
756+
const held = new Promise<void>((resolve) => {
757+
markHeld = resolve
758+
})
759+
let release = () => {}
760+
const released = new Promise<void>((resolve) => {
761+
release = resolve
762+
})
763+
const concurrentReader = db.transaction(async (tx) => {
764+
await lockProjectShared(tx, f.projectId)
765+
markHeld()
766+
await released
767+
})
768+
await held
769+
await clearSearchRevision(source.file.id)
770+
try {
771+
const started = performance.now()
772+
const rendered = await readProjectFileArtifact.execute({
773+
principal: f.principal,
774+
input: { projectId: f.projectId, fileId: source.file.id, maxBytes: 1_000_000 },
775+
})
776+
expect(rendered.buffer).toEqual(artifact)
777+
expect(performance.now() - started).toBeLessThan(
778+
FILE_SEARCH_ARTIFACT_READY_LOCK_TIMEOUT_MS + 2_000
779+
)
780+
} finally {
781+
release()
782+
await concurrentReader
783+
}
784+
expect(await searchRevision(source.file.id)).toBeUndefined()
785+
}
786+
)
787+
788+
check("a Project page embedding another owner's image is a missing input", async () => {
789+
const f = await fixture()
790+
for (const owner of [`workspace=${f.workspaceId}`, `project=${generateId()}`]) {
791+
const page = await createProjectFile.execute({
792+
principal: f.principal,
793+
input: {
794+
...createInput(
795+
f.projectId,
796+
`---\ntitle: Design\n---\n\n![Diagram](sim:file/${generateId()}?${owner})`
797+
),
798+
name: `${generateId()}.html`,
799+
contentType: 'text/x-sim-page',
800+
},
801+
})
802+
await expect(
803+
readProjectFileArtifact.execute({
804+
principal: f.principal,
805+
input: { projectId: f.projectId, fileId: page.file.id, maxBytes: 1_000_000 },
806+
})
807+
).rejects.toMatchObject({ code: 'not_found' })
808+
}
809+
})
810+
674811
check(
675812
'artifact source cannot import a foreign Project asset through a valid source-file capability',
676813
async () => {
@@ -846,6 +983,9 @@ afterAll(async () => {
846983
.where(
847984
sql`${outboxEvent.payload}::jsonb ->> 'key' LIKE ${`project/${f.projectId}/%`} OR ${outboxEvent.payload}::jsonb -> 'owner' ->> 'entityId' = ${f.projectId}`
848985
)
986+
await db
987+
.delete(fileSearchDispatchQueue)
988+
.where(eq(fileSearchDispatchQueue.entityId, f.projectId))
849989
await db.delete(workspaceFiles).where(eq(workspaceFiles.projectId, f.projectId))
850990
await db.delete(folder).where(eq(folder.projectId, f.projectId))
851991
await deleteWorkspaceFixture(db, eq(workspace.id, f.workspaceId))

0 commit comments

Comments
 (0)