Skip to content

Commit 1e45e2b

Browse files
fix(knowledge): restore processing takeover semantics
1 parent 5d95ca8 commit 1e45e2b

4 files changed

Lines changed: 20 additions & 67 deletions

File tree

apps/sim/app/api/knowledge/utils.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ describe('Knowledge Utils', () => {
175175
])
176176
/** In-transaction active-document recheck. */
177177
queueTableRows(schemaMock.document, [{ id: 'doc1' }])
178-
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'doc1' }])
179178

180179
await processDocumentAsync(
181180
'kb1',

apps/sim/lib/knowledge/documents/document-processing-source.test.ts

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,6 @@ describe('knowledge document processing source', () => {
138138
.mockResolvedValueOnce([PERSISTED_CONTEXT])
139139
.mockResolvedValueOnce([PERSISTED_PROVENANCE_ROW])
140140
.mockResolvedValueOnce([{ id: 'document-1' }])
141-
dbChainMockFns.returning.mockResolvedValueOnce([{ id: 'document-1' }])
142141
mockCheckActorUsageLimits.mockResolvedValue({ isExceeded: false })
143142
mockGetFileMetadataByKeys.mockImplementation(async (_keys: string[], context: string) =>
144143
context === 'workspace' ? [SOURCE_BINDING] : []
@@ -230,22 +229,28 @@ describe('knowledge document processing source', () => {
230229
expect(mockGenerateEmbeddings).not.toHaveBeenCalled()
231230
})
232231

233-
it('keeps the carrier retryable when another processing attempt owns the document', async () => {
232+
it('takes over an existing processing attempt', async () => {
233+
dbChainMockFns.limit
234+
.mockReset()
235+
.mockResolvedValueOnce([{ ...PERSISTED_CONTEXT, processingStatus: 'processing' }])
236+
.mockResolvedValueOnce([PERSISTED_PROVENANCE_ROW])
237+
.mockResolvedValueOnce([{ id: 'document-1' }])
234238
dbChainMockFns.returning.mockReset().mockResolvedValueOnce([])
235239

236-
await expect(
237-
processDocumentAsync('knowledge-base-1', 'document-1', {
238-
filename: 'stale.pdf',
239-
fileUrl: 'https://example.com/stale.pdf',
240-
fileSize: 1,
241-
mimeType: 'text/plain',
242-
})
243-
).rejects.toThrow('processing claim is owned by another attempt')
240+
await processDocumentAsync('knowledge-base-1', 'document-1', {
241+
filename: 'stale.pdf',
242+
fileUrl: 'https://example.com/stale.pdf',
243+
fileSize: 1,
244+
mimeType: 'text/plain',
245+
})
244246

245-
expect(mockProcessDocument).not.toHaveBeenCalled()
247+
expect(mockProcessDocument).toHaveBeenCalled()
246248
expect(mockGenerateEmbeddings).not.toHaveBeenCalled()
247-
expect(dbChainMockFns.set).not.toHaveBeenCalledWith(
248-
expect.objectContaining({ processingStatus: 'failed' })
249+
expect(dbChainMockFns.set).toHaveBeenCalledWith(
250+
expect.objectContaining({
251+
processingStatus: 'processing',
252+
processingStartedAt: expect.any(Date),
253+
})
249254
)
250255
})
251256
})

apps/sim/lib/knowledge/documents/processing-queue.test.ts

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ describe('processDocumentsWithQueue billing attribution', () => {
7272
beforeEach(() => {
7373
vi.clearAllMocks()
7474
resetDbChainMock()
75-
setEnvFlags({ isTriggerDevEnabled: true })
7675
mockBatchTrigger.mockResolvedValue({ batchId: 'batch-1' })
7776
for (const key of Object.keys(env)) {
7877
delete (env as Record<string, unknown>)[key]
@@ -153,29 +152,4 @@ describe('processDocumentsWithQueue billing attribution', () => {
153152
})
154153
expect(jobs[0].payload).not.toHaveProperty('billingAttribution')
155154
})
156-
157-
it('keeps direct dispatch retryable when another worker wins the document claim', async () => {
158-
setEnvFlags({ isTriggerDevEnabled: false })
159-
env.TRIGGER_SECRET_KEY = undefined
160-
dbChainMockFns.limit
161-
.mockResolvedValueOnce([{ userId: 'legacy-owner', workspaceId: null }])
162-
.mockResolvedValueOnce([
163-
{
164-
workspaceId: null,
165-
knowledgeBaseUserId: 'legacy-owner',
166-
processingStatus: 'pending',
167-
filename: DOCUMENT.filename,
168-
fileUrl: DOCUMENT.fileUrl,
169-
fileSize: DOCUMENT.fileSize,
170-
mimeType: DOCUMENT.mimeType,
171-
},
172-
])
173-
dbChainMockFns.returning.mockResolvedValueOnce([])
174-
175-
await expect(
176-
processDocumentsWithQueue([DOCUMENT], 'knowledge-base-1', {}, 'request-1', undefined)
177-
).rejects.toThrow('All 1 document processing dispatches failed')
178-
179-
expect(mockBatchTrigger).not.toHaveBeenCalled()
180-
})
181155
})

apps/sim/lib/knowledge/documents/service.ts

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,6 @@ import { calculateCost } from '@/providers/utils'
118118

119119
const logger = createLogger('DocumentService')
120120

121-
class DocumentProcessingClaimUnavailableError extends Error {
122-
constructor(documentId: string) {
123-
super(`Knowledge document ${documentId} processing claim is owned by another attempt`)
124-
this.name = 'DocumentProcessingClaimUnavailableError'
125-
}
126-
}
127-
128121
/**
129122
* Thrown when a knowledge-base document's `fileUrl` references an internal
130123
* knowledge-base storage object not owned by the target knowledge base's workspace.
@@ -824,7 +817,6 @@ export async function processDocumentAsync(
824817
boolean1: document.boolean1,
825818
boolean2: document.boolean2,
826819
boolean3: document.boolean3,
827-
processingStatus: document.processingStatus,
828820
})
829821
.from(document)
830822
.innerJoin(knowledgeBase, eq(knowledgeBase.id, document.knowledgeBaseId))
@@ -866,7 +858,7 @@ export async function processDocumentAsync(
866858
mimeType: ctx.mimeType,
867859
}
868860

869-
const [claimedDocument] = await db
861+
await db
870862
.update(document)
871863
.set({
872864
processingStatus: 'processing',
@@ -875,21 +867,8 @@ export async function processDocumentAsync(
875867
processingError: null,
876868
})
877869
.where(
878-
and(
879-
eq(document.id, documentId),
880-
inArray(document.processingStatus, ['pending', 'failed']),
881-
isNull(document.archivedAt),
882-
isNull(document.deletedAt)
883-
)
870+
and(eq(document.id, documentId), isNull(document.archivedAt), isNull(document.deletedAt))
884871
)
885-
.returning({ id: document.id })
886-
887-
if (!claimedDocument) {
888-
logger.info(`[${documentId}] Skipping document processing because another attempt owns it`, {
889-
processingStatus: ctx.processingStatus,
890-
})
891-
throw new DocumentProcessingClaimUnavailableError(documentId)
892-
}
893872

894873
logger.info(`[${documentId}] Status updated to 'processing', starting document processor`)
895874

@@ -1243,10 +1222,6 @@ export async function processDocumentAsync(
12431222
fileSize: docData.fileSize,
12441223
})
12451224

1246-
if (error instanceof DocumentProcessingClaimUnavailableError) {
1247-
throw error
1248-
}
1249-
12501225
await db
12511226
.update(document)
12521227
.set({

0 commit comments

Comments
 (0)