address comments

Author: icecrasher321

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/fork-activity-panel/fork-activity-panel.tsx

@@ -139,6 +139,9 @@ function jobReport(job: BackgroundWorkItem): JobReport {
   if (m.failed && m.failed > 0) {
     notes.push({ value: `${plural(m.failed, 'resource')} failed to copy`, warning: true })
   }
+  if (m.clearingFailed) {
+    notes.push({ value: 'Reference cleanup incomplete', warning: true })
+  }
   return { groups, notes }
 }
 

apps/sim/lib/api/contracts/workspace-fork.test.ts

@@ -12,14 +12,17 @@ describe('forkMappableResourceTypeSchema', () => {
     expect(forkMappableResourceTypeSchema.safeParse('workflow').success).toBe(false)
   })
 
+  it('rejects knowledge_document (a document follows its parent knowledge base)', () => {
+    expect(forkMappableResourceTypeSchema.safeParse('knowledge_document').success).toBe(false)
+  })
+
   it('accepts user-mappable resource types', () => {
     for (const type of [
       'oauth_credential',
       'service_account_credential',
       'env_var',
       'table',
       'knowledge_base',
-      'knowledge_document',
       'file',
       'mcp_server',
       'custom_tool',

apps/sim/lib/api/contracts/workspace-fork.ts

@@ -521,6 +521,8 @@ export const backgroundWorkMetadataSchema = z
     failed: z.number().int().optional(),
     /** Count of failed resources whose dangling references were cleared post-fork (U8). */
     clearedReferences: z.number().int().optional(),
+    /** True when a reference-clear phase threw, so cleanup is incomplete (placeholders not dropped). */
+    clearingFailed: z.boolean().optional(),
     /** Names of the resources a fork copied, by kind, for the report breakdown. */
     workflowNames: z.array(z.string()).optional(),
     tableNames: z.array(z.string()).optional(),

apps/sim/lib/workspaces/fork/copy/cleanup-failed.test.ts

@@ -334,7 +334,7 @@ describe('cleanup-failed', () => {
         requestId: 'test',
       })
 
-      expect(cleaned).toBe(1)
+      expect(cleaned).toEqual({ cleared: 1, clearingFailed: false })
       // One draft block update + one deployed version update (only the referencing version).
       const updatedTables = dbMock.updates.map((u) => u.table)
       expect(updatedTables).toEqual([workflowBlocks, workflowDeploymentVersion])
@@ -355,7 +355,7 @@ describe('cleanup-failed', () => {
         requestId: 'test',
       })
 
-      expect(cleaned).toBe(1)
+      expect(cleaned).toEqual({ cleared: 1, clearingFailed: false })
       // No draft block referenced the failed id AND no deployed targets were threaded, so the
       // deployed sweep is skipped entirely.
       expect(dbMock.updates).toHaveLength(0)
@@ -380,7 +380,7 @@ describe('cleanup-failed', () => {
         requestId: 'test',
       })
 
-      expect(cleaned).toBe(1)
+      expect(cleaned).toEqual({ cleared: 1, clearingFailed: false })
       expect(dbMock.updates.map((u) => u.table)).toContain(workflowDeploymentVersion)
       expect(mockInvalidateDeployedStateCache).toHaveBeenCalledWith('dv-1')
       // Clearing succeeded, so the placeholder is dropped.
@@ -397,7 +397,7 @@ describe('cleanup-failed', () => {
         requestId: 'test',
       })
 
-      expect(cleaned).toBe(1)
+      expect(cleaned).toEqual({ cleared: 1, clearingFailed: false })
       expect(dbMock.updates).toHaveLength(1)
       expect(dbMock.updates[0].table).toBe(workflowBlocks)
       const cleared = dbMock.updates[0].values.subBlocks as Record<string, { value: unknown }>
@@ -406,7 +406,7 @@ describe('cleanup-failed', () => {
       expect(dbMock.deletes).toHaveLength(0)
     })
 
-    it('skips the placeholder drop when a reference-clear phase throws', async () => {
+    it('reports cleared:0 + clearingFailed and skips the placeholder drop when a clear phase throws', async () => {
       // A clear-phase failure must not drop the placeholder: that would turn an empty placeholder
       // into a dangling reference to a deleted row. Make the draft block UPDATE throw.
       dbMock.queueRead(workflow, [{ id: 'wf-1' }])
@@ -421,7 +421,8 @@ describe('cleanup-failed', () => {
           failures: [{ kind: 'knowledge-base', childId: 'failed-kb', documentChildIds: [] }],
           requestId: 'test',
         })
-        expect(cleaned).toBe(1)
+        // The count must NOT overstate: nothing was cleared and the flag marks cleanup incomplete.
+        expect(cleaned).toEqual({ cleared: 0, clearingFailed: true })
       } finally {
         dbMock.db.update = originalUpdate
       }

apps/sim/lib/workspaces/fork/copy/cleanup-failed.ts

@@ -70,17 +70,21 @@ function clearFailedSubBlockReferences(
  * a cleanup error never aborts the others. The placeholder drop is SKIPPED when a reference-clear
  * phase threw - dropping it then would turn an empty placeholder into a dangling reference to a
  * deleted row; leaving it keeps the reference resolvable (to empty content) until a later retry.
- * Returns the number of failed resources cleaned, for the fork activity report.
+ *
+ * Returns `{ cleared, clearingFailed }` for the fork activity metadata. `clearingFailed` is true
+ * when a reference-clear phase threw - placeholders were then NOT dropped - and `cleared` is 0 in
+ * that case, so the report never claims references it did not actually clear. On success `cleared`
+ * is the count of failed resources whose references were cleared.
  */
 export async function clearFailedForkResourceReferences(params: {
   childWorkspaceId: string
   failures: ForkFailedResource[]
   /** Target workflows this sync deployed; their deployed versions are swept regardless of draft. */
   deployedTargetWorkflowIds?: string[]
   requestId?: string
-}): Promise<number> {
+}): Promise<{ cleared: number; clearingFailed: boolean }> {
   const { childWorkspaceId, failures, requestId = 'unknown' } = params
-  if (failures.length === 0) return 0
+  if (failures.length === 0) return { cleared: 0, clearingFailed: false }
 
   const failedByKind = new Map<ForkRemapKind, Set<string>>()
   const markFailed = (kind: ForkRemapKind, id: string) => {
@@ -166,7 +170,7 @@ export async function clearFailedForkResourceReferences(params: {
         documents: docIds.length,
       }
     )
-    return failures.length
+    return { cleared: 0, clearingFailed: true }
   }
   try {
     if (tableIds.length > 0) {
@@ -185,7 +189,7 @@ export async function clearFailedForkResourceReferences(params: {
     })
   }
 
-  return failures.length
+  return { cleared: failures.length, clearingFailed: false }
 }
 
 /**

apps/sim/lib/workspaces/fork/copy/content-copy-runner.ts

@@ -117,7 +117,7 @@ export async function runForkContentCopy(payload: ForkContentCopyPayload): Promi
       kind: 'file',
       childKey,
     }))
-    const clearedReferences = await clearFailedForkResourceReferences({
+    const { cleared, clearingFailed } = await clearFailedForkResourceReferences({
       childWorkspaceId: contentPlan.childWorkspaceId,
       failures: [...resourceCounts.failures, ...fileFailures],
       deployedTargetWorkflowIds: payload.deployedTargetWorkflowIds,
@@ -132,7 +132,12 @@ export async function runForkContentCopy(payload: ForkContentCopyPayload): Promi
           failed > 0
             ? `Copied ${copied} item${copied === 1 ? '' : 's'}; ${failed} could not be copied`
             : `Copied ${copied} item${copied === 1 ? '' : 's'}`,
-        metadata: { copied, failed, clearedReferences },
+        metadata: {
+          copied,
+          failed,
+          clearedReferences: cleared,
+          ...(clearingFailed ? { clearingFailed: true } : {}),
+        },
       })
     }
   } catch (error) {