Skip to content

Commit 88b4f45

Browse files
authored
fix(file-search): stop requeueing skipped artifacts, and fix public share, live-doc persist and cleanup edge cases (#9110)
* fix(file-search): keep a skipped build published so rendered reads stop requeueing it A skipped publication retired its build and cleared the revision's build id, so a compiled document whose index build skipped never matched its rendered artifact key: every rendered read requeued it, and the worker downloaded, compiled and skipped it again. Skipped builds now stay published with their bound artifact key (and no text), and the artifact-current checks accept a skipped revision. Failed revisions still requeue. * fix(public-shares): answer a shared Project page with a foreign embed as not found renderSimPageDocument throws ProjectPageForeignEmbedError for a Project page that embeds another owner's file. The authenticated artifact read maps it to not_found, but the public content and inline reads let it escape as a 500. Map it the same way at both public render sites. * fix(file-doc): let the workspace document persist take a full snapshot The relay's workspace persist ran behind the proxy with the default 10 MB body cap, so a collaborative state above ~7.5 MiB (12 MiB is allowed) was refused with 413 while the Project persist route took it. Exclude the workspace persist from the proxy matcher and give it the same bypassesProxy body cap, now shared from collab-doc state, and the same off-app-host refusal. * chore(knip): drop the stale ignore for the v2 Project file search contract Its only export is now imported by the v2 route and the OpenAPI spec. * fix(memory): let a rejected key spend memory cleanup attempts A 401 from the worker waited hourly without consuming an attempt, like a missing route, so a permanently revoked self-hosted key kept its cleanup rows pending forever. A 401 still waits an hour but now spends an attempt, so it dead-letters after the default attempts (about ten hours), past a worker rollout. 404, 405 and 501 keep waiting for free. * fix(public-shares): log expected public file share client errors as warnings The public file share routes logged every caught error at ERROR, including classified client failures such as a missing share (404) or a password exchange against a non-password share (400). Log a classified 4xx as a warning and keep ERROR for unclassified failures. * fix(public-shares): import the error logger in the OTP route * test(file-search): skip on a corrupt compiled document the parsers reject
1 parent 62f609b commit 88b4f45

19 files changed

Lines changed: 162 additions & 41 deletions

File tree

‎apps/sim/app/api/files/public/[token]/content/route.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { parseRequest } from '@/lib/api/server'
55
import { getClientIp } from '@/lib/core/utils/request'
66
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
77
import {
8+
logPublicFileError,
89
publicFileAuthDenied,
910
publicFileBinaryErrorResponse,
1011
publicFileShareCredential,
@@ -58,7 +59,7 @@ export const GET = withRouteHandler(
5859
request.headers.get('if-none-match')
5960
)
6061
} catch (error) {
61-
logger.error('Error serving public shared file:', error)
62+
logPublicFileError(logger, 'Error serving public shared file:', error)
6263
return publicFileBinaryErrorResponse(error, 'Failed to serve file')
6364
}
6465
}

‎apps/sim/app/api/files/public/[token]/inline/route.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { parseRequest } from '@/lib/api/server'
55
import { getClientIp } from '@/lib/core/utils/request'
66
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
77
import {
8+
logPublicFileError,
89
publicFileAuthDenied,
910
publicFileBinaryErrorResponse,
1011
publicFileShareCredential,
@@ -48,7 +49,7 @@ export const GET = withRouteHandler(
4849
request.headers.get('if-none-match')
4950
)
5051
} catch (error) {
51-
logger.error('Error serving public inline image:', error)
52+
logPublicFileError(logger, 'Error serving public inline image:', error)
5253
return publicFileBinaryErrorResponse(error, 'Failed to serve file')
5354
}
5455
}

‎apps/sim/app/api/files/public/[token]/otp/route.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@ import { OTP_IP_RATE_LIMIT } from '@/lib/core/security/otp'
1111
import { afterResponse } from '@/lib/core/utils/after-response'
1212
import { getClientIp } from '@/lib/core/utils/request'
1313
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
14-
import { publicFileAuthDenied, publicFileErrorResponse } from '@/lib/public-shares/api'
14+
import {
15+
logPublicFileError,
16+
publicFileAuthDenied,
17+
publicFileErrorResponse,
18+
} from '@/lib/public-shares/api'
1519
import {
1620
requestPublicFileShareOtp,
1721
verifyPublicFileShareOtp,
@@ -48,7 +52,7 @@ export const POST = withRouteHandler(
4852
afterResponse(accepted.deliver)
4953
return NextResponse.json({ message: 'Verification code sent' })
5054
} catch (error) {
51-
logger.error('Error processing OTP request:', error)
55+
logPublicFileError(logger, 'Error processing OTP request:', error)
5256
return publicFileErrorResponse(error, 'Failed to process request')
5357
}
5458
}
@@ -69,7 +73,7 @@ export const PUT = withRouteHandler(
6973
response.cookies.set(auth.cookie)
7074
return response
7175
} catch (error) {
72-
logger.error('Error verifying OTP:', error)
76+
logPublicFileError(logger, 'Error verifying OTP:', error)
7377
return publicFileErrorResponse(error, 'Failed to process request')
7478
}
7579
}

‎apps/sim/app/api/files/public/[token]/route.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { parseRequest } from '@/lib/api/server'
99
import { getClientIp } from '@/lib/core/utils/request'
1010
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1111
import {
12+
logPublicFileError,
1213
publicFileAuthDenied,
1314
publicFileErrorResponse,
1415
publicFileShareCredential,
@@ -48,7 +49,7 @@ export const GET = withRouteHandler(
4849
ownerName,
4950
})
5051
} catch (error) {
51-
logger.error('Error fetching public file metadata:', error)
52+
logPublicFileError(logger, 'Error fetching public file metadata:', error)
5253
return publicFileErrorResponse(error, 'Failed to fetch file')
5354
}
5455
}
@@ -70,7 +71,7 @@ export const POST = withRouteHandler(
7071
response.cookies.set(auth.cookie)
7172
return response
7273
} catch (error) {
73-
logger.error('Error authenticating public file share:', error)
74+
logPublicFileError(logger, 'Error authenticating public file share:', error)
7475
return publicFileErrorResponse(error, 'Failed to authenticate')
7576
}
7677
}

‎apps/sim/app/api/files/public/[token]/sso/route.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,11 @@ import { parseRequest } from '@/lib/api/server'
66
import { RateLimiter, type TokenBucketConfig } from '@/lib/core/rate-limiter'
77
import { getClientIp } from '@/lib/core/utils/request'
88
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
9-
import { publicFileAuthDenied, publicFileErrorResponse } from '@/lib/public-shares/api'
9+
import {
10+
logPublicFileError,
11+
publicFileAuthDenied,
12+
publicFileErrorResponse,
13+
} from '@/lib/public-shares/api'
1014
import { getPublicFileShareSsoEligibility } from '@/lib/public-shares/application'
1115

1216
export const dynamic = 'force-dynamic'
@@ -47,7 +51,7 @@ export const POST = withRouteHandler(
4751
? NextResponse.json({ eligible: result.eligible })
4852
: publicFileAuthDenied(result)
4953
} catch (error) {
50-
logger.error('Error checking public file SSO eligibility:', error)
54+
logPublicFileError(logger, 'Error checking public file SSO eligibility:', error)
5155
return publicFileErrorResponse(error, 'Failed to process request')
5256
}
5357
}

‎apps/sim/app/api/internal/file-doc/persist/route.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import { getErrorMessage } from '@sim/utils/errors'
33
import type { NextRequest } from 'next/server'
44
import { NextResponse } from 'next/server'
55
import { persistFileDocContract } from '@/lib/api/contracts/file-doc'
6+
import { isOffAppHost } from '@/lib/api/mcp/host-routing'
67
import { parseRequest } from '@/lib/api/server'
78
import { persistFileDoc } from '@/lib/collab-doc/persist'
9+
import { MAX_COLLAB_DOC_PERSIST_BODY_BYTES } from '@/lib/collab-doc/state'
810
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
911
import { checkInternalApiKey, createUnauthorizedResponse } from '@/lib/mothership/request/http'
1012

@@ -16,12 +18,19 @@ const logger = createLogger('FileDocPersistAPI')
1618
* the shared `x-api-key: INTERNAL_API_SECRET` secret, matching the header the realtime relay sends.
1719
* The relay owns the live doc but not the conversion engine or blob/DB access, so it ships the current
1820
* doc state here — the server-authoritative durable path that replaces the editor's client autosave.
21+
* Excluded from the proxy matcher, so the route refuses the dedicated MCP host before auth itself.
1922
*/
2023
export const POST = withRouteHandler(async (request: NextRequest) => {
24+
if (isOffAppHost(request)) return new NextResponse(null, { status: 404 })
2125
const auth = checkInternalApiKey(request)
2226
if (!auth.success) return createUnauthorizedResponse()
2327

24-
const parsed = await parseRequest(persistFileDocContract, request, {})
28+
const parsed = await parseRequest(
29+
persistFileDocContract,
30+
request,
31+
{},
32+
{ maxBodyBytes: MAX_COLLAB_DOC_PERSIST_BODY_BYTES, bypassesProxy: true }
33+
)
2534
if (!parsed.success) return parsed.response
2635
const { workspaceId, fileId, userId, docState, expectedVersion } = parsed.data.body
2736

‎apps/sim/app/api/internal/project-file-doc/[projectId]/[fileId]/persist/route.ts‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,9 @@ import {
77
internalRateLimits,
88
} from '@/lib/api/server/routes'
99
import { realtimeProjectFileAuth } from '@/lib/auth/realtime-file-delegation'
10-
import { MAX_COLLAB_DOC_STATE_BYTES } from '@/lib/collab-doc/state'
10+
import { MAX_COLLAB_DOC_PERSIST_BODY_BYTES } from '@/lib/collab-doc/state'
1111
import { persistProjectFileDoc } from '@/lib/projects/files/application/documents'
1212

13-
/** A base64-encoded maximum snapshot plus 1 MiB for its JSON envelope (17 MiB). */
14-
const PERSIST_MAX_BODY_BYTES = Math.ceil(MAX_COLLAB_DOC_STATE_BYTES / 3) * 4 + 1024 * 1024
15-
1613
const persist = defineInternalJsonRoute({
1714
contract: projectFileDocPersistContract,
1815
operation: persistProjectFileDoc.operation,
@@ -27,7 +24,7 @@ const persist = defineInternalJsonRoute({
2724
expectedVersion: body.expectedVersion,
2825
}),
2926
useCase: persistProjectFileDoc,
30-
parseOptions: { maxBodyBytes: PERSIST_MAX_BODY_BYTES, bypassesProxy: true },
27+
parseOptions: { maxBodyBytes: MAX_COLLAB_DOC_PERSIST_BODY_BYTES, bypassesProxy: true },
3128
})
3229

3330
/** Excluded from the proxy matcher, so the route refuses the dedicated MCP host before auth itself. */

‎apps/sim/lib/collab-doc/state.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@ import type { DbOrTx, DbTransaction } from '@/lib/db/types'
77
/** Matches the decoded size of the persist endpoint's 16 MiB base64 snapshot limit. */
88
export const MAX_COLLAB_DOC_STATE_BYTES = 12 * 1024 * 1024
99

10+
/**
11+
* Body cap of the relay's persist routes: a base64-encoded maximum snapshot plus 1 MiB for its JSON
12+
* envelope (17 MiB). Both routes bypass the proxy, whose 10 MB buffer would truncate it.
13+
*/
14+
export const MAX_COLLAB_DOC_PERSIST_BODY_BYTES =
15+
Math.ceil(MAX_COLLAB_DOC_STATE_BYTES / 3) * 4 + 1024 * 1024
16+
1017
/** Reject oversized snapshots before copying, decoding, or writing them. */
1118
export function assertCollabDocStateSize(docState: Uint8Array): void {
1219
if (docState.byteLength > MAX_COLLAB_DOC_STATE_BYTES) {

‎apps/sim/lib/mothership/memories/cleanup-outbox.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,15 @@ const logger = createLogger('MemoryCleanup')
2929
const UNAVAILABLE_RETRY_MS = 60 * 60 * 1000
3030
/**
3131
* Statuses meaning the worker does not serve cleanup yet, not that this request failed: a worker
32-
* that predates the route answers 404 to the hosted key and 401 to a self-hosted customer key.
32+
* that predates the route answers 404 to the hosted key.
3333
*/
34-
const ENDPOINT_MISSING_STATUSES = new Set([401, 404, 405, 501])
34+
const ENDPOINT_MISSING_STATUSES = new Set([404, 405, 501])
35+
/**
36+
* A worker that predates the route also answers this to a self-hosted customer key, but so does a
37+
* worker that revoked the key for good: it waits like a missing route but spends an attempt, so a
38+
* revoked key dead-letters after the default attempts (about ten hours), past a worker rollout.
39+
*/
40+
const UNAUTHORIZED_STATUS = 401
3541

3642
/** Non-strict so rows enqueued with the retired `credentialFingerprint` field still parse. */
3743
const cleanupPayloadSchema = z.object({
@@ -74,16 +80,21 @@ export async function enqueueMemoryCleanup(
7480
}
7581
}
7682

77-
function waitForWorker(reason: string, eventId: string, detail?: Record<string, string>) {
83+
function waitForWorker(
84+
reason: string,
85+
eventId: string,
86+
{ consumeAttempt = false, ...detail }: { baseUrl?: string; consumeAttempt?: boolean } = {}
87+
) {
7888
logger.warn(`Memory cleanup waiting: ${reason}`, { eventId, ...detail })
79-
return deferOutboxHandler(reason, UNAVAILABLE_RETRY_MS, false)
89+
return deferOutboxHandler(reason, UNAVAILABLE_RETRY_MS, consumeAttempt)
8090
}
8191

8292
/**
8393
* Sends the processor's current key only to a destination the processor itself configures, and
8494
* replays the enqueuing deployment's source env so the worker erases that deployment's notes.
8595
* A missing key, destination, or worker endpoint waits without consuming attempts: erasure must
86-
* not dead-letter while configuration or the worker rollout catches up.
96+
* not dead-letter while configuration or the worker rollout catches up. A rejected key waits too,
97+
* but within the retry budget.
8798
*/
8899
export const memoryCleanupOutboxHandlers = {
89100
[MEMORY_CLEANUP_EVENT]: async (rawPayload, context) => {
@@ -109,6 +120,10 @@ export const memoryCleanupOutboxHandlers = {
109120
await response.body?.cancel().catch(() => {})
110121
if (ENDPOINT_MISSING_STATUSES.has(response.status))
111122
return waitForWorker(`endpoint unavailable (${response.status})`, context.eventId)
123+
if (response.status === UNAUTHORIZED_STATUS)
124+
return waitForWorker('worker rejected the key (401)', context.eventId, {
125+
consumeAttempt: true,
126+
})
112127
throw new Error(`Memory cleanup failed (${response.status})`)
113128
}
114129
MemoryCleanupResult.parse(await response.json())

‎apps/sim/lib/mothership/memories/cleanup.integration.ts‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,10 @@ it('rolls cleanup back with deletion and retries committed account/target erasur
139139
expect(received.every((request) => request.key === 'memory-cleanup-fixture-key')).toBe(true)
140140

141141
const first = events[0]
142-
const replay = async (payload: unknown = first.payload) => {
142+
const replay = async (payload: unknown = first.payload, attempts = 9) => {
143143
await db
144144
.update(outboxEvent)
145-
.set({ status: 'pending', availableAt: new Date(0), attempts: 9, payload })
145+
.set({ status: 'pending', availableAt: new Date(0), attempts, payload })
146146
.where(eq(outboxEvent.id, first.id))
147147
await runtime.outbox.processOutboxEventById(
148148
first.id,
@@ -161,11 +161,18 @@ it('rolls cleanup back with deletion and retries committed account/target erasur
161161
waitsAnHour(await replay())
162162
expect(received).toHaveLength(before + 1)
163163

164+
status = 401
165+
const rejected = await replay(first.payload, 5)
166+
expect(rejected).toMatchObject({ status: 'pending', attempts: 6 })
167+
expect(rejected.availableAt.getTime()).toBeGreaterThan(Date.now() + 50 * 60_000)
168+
expect(await replay()).toMatchObject({ status: 'dead_letter', attempts: 10 })
169+
expect(received).toHaveLength(before + 3)
170+
164171
status = 200
165172
waitsAnHour(await replay({ ...toRecord(first.payload), baseUrl: 'https://unconfigured.test' }))
166-
expect(received).toHaveLength(before + 1)
173+
expect(received).toHaveLength(before + 3)
167174

168175
const legacy = await replay({ ...toRecord(first.payload), credentialFingerprint: 'retired' })
169176
expect(legacy.status).toBe('completed')
170-
expect(received).toHaveLength(before + 2)
177+
expect(received).toHaveLength(before + 4)
171178
}, 30_000)

0 commit comments

Comments
 (0)