Skip to content

Commit a618053

Browse files
committed
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
1 parent 2916f4a commit a618053

13 files changed

Lines changed: 565 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: 132 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

@@ -306,6 +310,7 @@ describe('Project delivery admission', () => {
306310
}
307311
vi.spyOn(searchRepository, 'searchFileIndex').mockResolvedValue(prepared)
308312
vi.spyOn(searchDelivery, 'loadFileSearchDelivery').mockResolvedValue({
313+
current: new Set(),
309314
provenance: new Map(),
310315
outOfScope: new Set(['moved']),
311316
inputs: [],
@@ -671,6 +676,130 @@ describe('Project rendered artifacts against PostgreSQL and private storage', ()
671676
}
672677
)
673678

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

0 commit comments

Comments
 (0)