Skip to content

Commit 8cb535d

Browse files
committed
fix fork copies to work with provenance
1 parent 043a152 commit 8cb535d

18 files changed

Lines changed: 1791 additions & 269 deletions

apps/sim/ee/workspace-forking/lib/copy/cleanup-failed.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,31 @@ describe('cleanup-failed', () => {
269269
expect(updates()).toHaveLength(0)
270270
expect(mockInvalidateDeployedStateCache).not.toHaveBeenCalled()
271271
})
272+
273+
it('attempts every workflow, then reports a workflow-scoped cleanup failure', async () => {
274+
queueTableRows(workflowDeploymentVersion, [
275+
{ id: 'dv-failed', version: 5, state: versionState('failed-kb') },
276+
])
277+
queueTableRows(workflowDeploymentVersion, [
278+
{ id: 'dv-cleaned', version: 5, state: versionState('failed-kb') },
279+
])
280+
dbChainMockFns.set.mockImplementationOnce(() => {
281+
throw new Error('first workflow update failed')
282+
})
283+
284+
await expect(
285+
clearFailedReferencesInDeploymentVersions(
286+
new Set(['wf-failed', 'wf-cleaned']),
287+
failedByKind(),
288+
'test'
289+
)
290+
).rejects.toThrow('Failed to clear deployment-version references for 1 workflow(s)')
291+
292+
// The second workflow is still processed after the first workflow's update fails.
293+
expect(dbChainMockFns.update).toHaveBeenCalledTimes(2)
294+
expect(mockInvalidateDeployedStateCache).toHaveBeenCalledTimes(1)
295+
expect(mockInvalidateDeployedStateCache).toHaveBeenCalledWith('dv-cleaned')
296+
})
272297
})
273298

274299
describe('clearFailedForkResourceReferences', () => {
@@ -376,5 +401,26 @@ describe('cleanup-failed', () => {
376401
// The drop is skipped, so the placeholder row survives (no delete issued).
377402
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
378403
})
404+
405+
it('keeps placeholders when a deployed-version cleanup fails after draft cleanup succeeds', async () => {
406+
queueTableRows(workflow, [{ id: 'wf-1' }])
407+
queueTableRows(workflowBlocks, [draftBlockRow('other-kb')])
408+
queueTableRows(workflowDeploymentVersion, [
409+
{ id: 'dv-failed', version: 5, state: versionState('failed-kb') },
410+
])
411+
dbChainMockFns.set.mockImplementationOnce(() => {
412+
throw new Error('deployment update failed')
413+
})
414+
415+
const cleaned = await clearFailedForkResourceReferences({
416+
childWorkspaceId: 'child-ws',
417+
failures: [{ kind: 'knowledge-base', childId: 'failed-kb', documentChildIds: [] }],
418+
deployedTargetWorkflowIds: ['wf-deployed'],
419+
requestId: 'test',
420+
})
421+
422+
expect(cleaned).toEqual({ cleared: 0, clearingFailed: true })
423+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
424+
})
379425
})
380426
})

apps/sim/ee/workspace-forking/lib/copy/cleanup-failed.ts

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -77,12 +77,9 @@ function clearFailedSubBlockReferences(
7777
* is the count of failed resources whose references were cleared.
7878
*
7979
* Storage accounting: this cleanup never decrements storage usage because it never removes
80-
* anything that was counted. Copied file blobs are the only counted copies (incremented in
81-
* `executeForkFileBlobCopies` only after the blob lands), and a failed file's blob never
82-
* landed - its metadata row is intentionally left re-uploadable, and nothing was charged. The
83-
* dropped table/KB/document placeholders are DB rows the upload path never counts, and any KB
84-
* blobs copied before their KB failed are left in storage (rows only are dropped here) but
85-
* uncounted - mirroring the KB upload path, which never counts KB blobs.
80+
* anything that remains counted. A failed file copy is not charged and leaves its metadata row
81+
* re-uploadable. A failed KB copy reverses its usage and retires its active file-ownership rows
82+
* before reaching this cleanup; deterministic blobs remain available for a safe retry.
8683
*/
8784
export async function clearFailedForkResourceReferences(params: {
8885
childWorkspaceId: string
@@ -304,6 +301,9 @@ export function rewriteDeploymentVersionState(
304301
* no-op. After a version is rewritten its cached deployed state is evicted so execute/serve rebuilds
305302
* from the cleaned snapshot. Bounded work (no long transaction): per-version short UPDATEs, versions
306303
* keyset-paginated, and a per-workflow failure is logged without aborting the other workflows.
304+
* After every workflow has been attempted, any failures are reported to the caller so it can keep
305+
* the failed resource placeholders in place rather than deleting rows a deployed version may still
306+
* reference.
307307
*/
308308
export async function clearFailedReferencesInDeploymentVersions(
309309
workflowIds: ReadonlySet<string>,
@@ -312,6 +312,7 @@ export async function clearFailedReferencesInDeploymentVersions(
312312
): Promise<void> {
313313
if (workflowIds.size === 0) return
314314
const resolve = buildFailedResolver(failedByKind)
315+
const failures: unknown[] = []
315316

316317
for (const workflowId of workflowIds) {
317318
try {
@@ -355,10 +356,18 @@ export async function clearFailedReferencesInDeploymentVersions(
355356
afterVersion = versions[versions.length - 1].version
356357
}
357358
} catch (error) {
359+
failures.push(error)
358360
logger.error(`[${requestId}] Failed to clear references in deployment versions`, {
359361
workflowId,
360362
error: getErrorMessage(error),
361363
})
362364
}
363365
}
366+
367+
if (failures.length > 0) {
368+
throw new AggregateError(
369+
failures,
370+
`Failed to clear deployment-version references for ${failures.length} workflow(s)`
371+
)
372+
}
364373
}

0 commit comments

Comments
 (0)