Skip to content

Commit 9335ba5

Browse files
icecrasher321claude
andcommitted
fix(forks): stop copying connector-managed knowledge base documents
A fork copies a KB's documents but never its connectors, so a connector-sourced document arrives with `connector_id` nulled and its `external_id` intact. The sync engine keys every existing/tombstone/ exclusion lookup off `connector_id`, so that copy is invisible to it - never updated, reconciled, or purged - and `doc_connector_external_id_idx` does not constrain it either, since its `connector_id` is NULL. Attaching a connector in the child then re-ingests every page as a NEW row on top of the snapshot. Each fork hop re-copies the previous hop's orphans and adds one more generation, so a prod -> UAT -> staging chain leaves three rows per page and a knowledge search returns the same page three times, one of them serving content frozen at the fork date. Exclude connector-managed documents from all four doors a document can enter a fork through: the whole-KB content copy, the in-transaction placeholder pre-creation, the sync-only copy into an already-mapped KB, and the content fill (guarded for payloads planned by a pre-change worker mid-rollout). The placeholder path matters as much as the copy loop - filtering only the content phase would leave a permanently archived row behind a persisted `knowledge_document` mapping. Skipped on both sides, the reference clears like any other uncopied document's. A document whose connector was deleted already has a null `connector_id` (the FK is ON DELETE SET NULL) and is static in the source too, so it still copies. One count(*) per copied KB logs what was left behind, since a fully connector-synced KB now forks to zero documents. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 1c69372 commit 9335ba5

3 files changed

Lines changed: 191 additions & 9 deletions

File tree

apps/docs/content/docs/en/platform/enterprise/forks.mdx

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ How each resource behaves at **fork** time vs **sync** time. Use this when you a
174174
| [Excluded workflows](#excluded-workflows) | Never | Never — not sent, not overwritten, not archived |
175175
| Files | Optional copy (default on) | Map or copy |
176176
| Tables | Optional copy (default on) | Map or copy |
177-
| Knowledge bases (+ documents) | Optional copy; referenced docs come with the KB | Map or copy; documents follow the KB |
177+
| Knowledge bases (+ documents) | Optional copy; uploaded documents come with the KB, [connector-synced ones do not](#connector-synced-documents-are-not-copied) | Map or copy; documents follow the KB |
178178
| Custom tools | Optional copy (default on) | Map or copy |
179179
| Skills | Optional copy (default on) | Map or copy |
180180
| External MCP servers | Optional copy (config only; sign-in cleared) | Map or copy (config only; sign-in cleared) |
@@ -227,10 +227,27 @@ Only **deployed** workflows move. Deploy is the commit; sync is the force push/p
227227

228228
| | Behavior |
229229
|---|----------|
230-
| **Fork** | Optional copy (default on). Tag definitions come with the knowledge base. Documents that the forked workflows actually reference are included. Deselect → knowledge base / document fields clear. |
230+
| **Fork** | Optional copy (default on). Tag definitions come with the knowledge base, along with every **uploaded** document in it. Deselect → knowledge base / document fields clear. |
231231
| **Sync** | Map or copy the knowledge base. Documents are not mapped by themselves — they follow the knowledge base (copied with it, or re-picked when you map to an existing one). |
232232

233-
**Example:** An agent searches knowledge base “Product docs.” Fork with that knowledge base selected → the child gets the base, tags, and the documents the agent used. On sync, mapping to the child’s existing “Product docs” means re-picking which document the tool should use.
233+
**Example:** An agent searches knowledge base “Product docs.” Fork with that knowledge base selected → the child gets the base, tags, and the uploaded documents. On sync, mapping to the child’s existing “Product docs” means re-picking which document the tool should use.
234+
235+
#### Connector-synced documents are not copied
236+
237+
Connectors themselves never cross a fork edge — the child gets no Confluence, Notion, Google Drive, or other sync running against it. Documents that a **connector** put in the knowledge base are therefore not copied either. Only documents you **uploaded** come across.
238+
239+
<Callout type="warn">
240+
Fork a knowledge base whose content is entirely connector-synced and the child gets the base, its tags, and its settings — but **no documents**. Add the connector in the child to fill it.
241+
</Callout>
242+
243+
This is deliberate. A copied connector document would arrive detached from any connector, so nothing would ever update, re-sync, or remove it — and when you added the connector in the child it would ingest every page again *alongside* the stale copy. Chain a few forks (prod → UAT → staging) and each hop leaves another dead generation behind, so one page comes back several times in a single knowledge search. Skipping them keeps the child’s own connector the single owner of that content.
244+
245+
| To get connector content into the child | Do this |
246+
|---|---|
247+
| Keep it live | Add the same connector in the child and let it sync. It re-ingests everything, so nothing is lost. |
248+
| Keep a frozen snapshot | Download the documents from the source and upload them to the child’s knowledge base — uploaded documents copy on every later fork. |
249+
250+
A document whose connector was **deleted** in the source is no longer connector-managed, so it copies like any other uploaded document.
234251

235252
---
236253

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

Lines changed: 108 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@ import { folder as folderTable } from '@sim/db/schema'
66
import { sha256Hex } from '@sim/security/hash'
77
import {
88
dbChainMockFns,
9+
flattenMockConditions,
910
resetDbChainMock,
11+
schemaMock,
1012
storageServiceMock,
1113
storageServiceMockFns,
1214
} from '@sim/testing'
@@ -311,6 +313,28 @@ describe('copyForkResourceContent', () => {
311313
expect(mockPersistCopiedResourceMappings).not.toHaveBeenCalled()
312314
})
313315

316+
it('never copies a connector-managed document out of the source knowledge base', async () => {
317+
dbChainMockFns.limit.mockResolvedValueOnce([])
318+
319+
const result = await copyForkResourceContent({
320+
contentPlan: basePlan({
321+
knowledgeBases: [{ sourceId: 'src-kb', childId: 'child-kb', documentIdMap: {} }],
322+
}),
323+
requestId: 'test',
324+
})
325+
326+
expect(result).toEqual({ copied: 1, failed: 0, failures: [] })
327+
// The row queue returns whatever is enqueued regardless of the predicate, so the exclusion
328+
// is only observable in the condition tree. Pinned to the column so the assertion keeps its
329+
// meaning if another nullable filter joins the same clause.
330+
const pageWhere = dbChainMockFns.where.mock.calls.at(-1)?.[0]
331+
expect(
332+
flattenMockConditions(pageWhere).some(
333+
(node) => node.type === 'isNull' && node.column === schemaMock.document.connectorId
334+
)
335+
).toBe(true)
336+
})
337+
314338
it('uses the blob content digest so a retry cannot adopt an older failed snapshot', async () => {
315339
dbChainMockFns.limit
316340
.mockResolvedValueOnce([sourceDoc])
@@ -1051,6 +1075,25 @@ describe('copyForkResourceContent', () => {
10511075
})
10521076
})
10531077

1078+
it('U-docs: refuses a connector-managed source planned before the exclusion existed', async () => {
1079+
// A payload queued by a pre-change worker during a rolling deploy: the planner would no
1080+
// longer emit this entry, so the fill must drop the placeholder rather than detach a copy
1081+
// of a connector-managed document into the existing target KB.
1082+
dbChainMockFns.limit
1083+
.mockResolvedValueOnce([])
1084+
.mockResolvedValueOnce([{ ...sourceDoc, connectorId: 'connector-1' }])
1085+
1086+
const result = await copyForkResourceContent({
1087+
contentPlan: mappedDocumentPlan(),
1088+
requestId: 'test',
1089+
})
1090+
1091+
expect(result.copied).toBe(0)
1092+
expect(result.failures).toEqual([{ kind: 'knowledge-document', childId: 'child-doc-1' }])
1093+
expect(storageServiceMockFns.mockDownloadFile).not.toHaveBeenCalled()
1094+
expect(mockIncrementStorageUsageInTx).not.toHaveBeenCalled()
1095+
})
1096+
10541097
it('U-docs: refuses to charge when the target knowledge base moved workspaces', async () => {
10551098
queueMappedDocumentCopy()
10561099
dbChainMockFns.for.mockResolvedValueOnce([{ workspaceId: 'other-workspace' }])
@@ -1359,10 +1402,12 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => {
13591402
// would make every source folder look already-present and suppress the mirroring.
13601403
let folderCall = 0
13611404
const inserts: Array<Array<Record<string, unknown>>> = []
1405+
const wheres: Array<{ table: unknown; condition: unknown }> = []
13621406
const tx = {
13631407
select: () => ({
13641408
from: (table: unknown) => ({
1365-
where: () => {
1409+
where: (condition: unknown) => {
1410+
wheres.push({ table, condition })
13661411
if (table === folderTable) {
13671412
return Promise.resolve(folderCall++ === 0 ? sourceFolders : [])
13681413
}
@@ -1377,7 +1422,7 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => {
13771422
},
13781423
}),
13791424
}
1380-
return { tx: tx as unknown as DbOrTx, inserts }
1425+
return { tx: tx as unknown as DbOrTx, inserts, wheres }
13811426
}
13821427

13831428
const kbSelection = {
@@ -1458,6 +1503,35 @@ describe('copyForkResourceContainers knowledge-base tag definitions', () => {
14581503
expect(inserts).toHaveLength(1)
14591504
})
14601505

1506+
it('does not pre-create a placeholder for a referenced connector-managed document', async () => {
1507+
const { tx, wheres } = makeKbTx([[sourceBase], [], []])
1508+
1509+
const result = await copyForkResourceContainers({
1510+
tx,
1511+
sourceWorkspaceId: 'src-ws',
1512+
childWorkspaceId: 'child-ws',
1513+
userId: 'user-1',
1514+
now: new Date(),
1515+
selection: kbSelection,
1516+
workflowIdMap: new Map(),
1517+
referencedDocumentIds: ['doc-1'],
1518+
documentMappingContext: { edgeChildWorkspaceId: 'child-ws', sourceIsParent: true },
1519+
})
1520+
1521+
// Must agree with the content phase's exclusion: a placeholder with no content copy behind
1522+
// it would stay archived forever while its persisted mapping pointed at it.
1523+
const placeholderWhere = wheres.find(({ table }) => table === schemaMock.document)?.condition
1524+
expect(
1525+
flattenMockConditions(placeholderWhere).some(
1526+
(node) => node.type === 'isNull' && node.column === schemaMock.document.connectorId
1527+
)
1528+
).toBe(true)
1529+
expect(result.mappingEntries.some((entry) => entry.resourceType === 'knowledge_document')).toBe(
1530+
false
1531+
)
1532+
expect(result.contentPlan.knowledgeBases[0].documentIdMap).toEqual({})
1533+
})
1534+
14611535
it('mirrors the source knowledge-base folder and copies the KB into it, not the target root', async () => {
14621536
const foldered = { ...sourceBase, folderId: 'kb-folder' }
14631537
const { tx, inserts } = makeKbTx(
@@ -1510,7 +1584,9 @@ describe('planForkMappedKbDocumentCopies', () => {
15101584
fileSize: 123,
15111585
filename: `${id}.pdf`,
15121586
mimeType: 'application/pdf',
1513-
connectorId: 'connector-1',
1587+
// Hand-uploaded: connector-managed documents are filtered out by the candidate query and
1588+
// can never reach the placeholder insert.
1589+
connectorId: null,
15141590
deletedAt: null,
15151591
archivedAt: null,
15161592
})
@@ -1526,11 +1602,19 @@ describe('planForkMappedKbDocumentCopies', () => {
15261602
}> = []
15271603
) {
15281604
const inserted: Array<Record<string, unknown>> = []
1605+
const wheres: unknown[] = []
15291606
let selectCalls = 0
15301607
const tx = {
15311608
select: () => {
15321609
const rows = selectCalls++ === 0 ? docs : existingTargets
1533-
return { from: () => ({ where: () => Promise.resolve(rows) }) }
1610+
return {
1611+
from: () => ({
1612+
where: (condition: unknown) => {
1613+
wheres.push(condition)
1614+
return Promise.resolve(rows)
1615+
},
1616+
}),
1617+
}
15341618
},
15351619
insert: () => ({
15361620
values: (rows: Array<Record<string, unknown>>) => {
@@ -1539,7 +1623,7 @@ describe('planForkMappedKbDocumentCopies', () => {
15391623
},
15401624
}),
15411625
}
1542-
return { tx: tx as unknown as DbOrTx, inserted, selectCalls: () => selectCalls }
1626+
return { tx: tx as unknown as DbOrTx, inserted, wheres, selectCalls: () => selectCalls }
15431627
}
15441628

15451629
const mappedKbResolver: ForkReferenceResolver = (kind, id) =>
@@ -1584,6 +1668,25 @@ describe('planForkMappedKbDocumentCopies', () => {
15841668
])
15851669
})
15861670

1671+
it('never considers a connector-managed doc as a candidate for the mapped target KB', async () => {
1672+
const { tx, wheres } = makeTx([])
1673+
await planForkMappedKbDocumentCopies({
1674+
tx,
1675+
resolver: mappedKbResolver,
1676+
referencedDocumentIds: ['doc-1'],
1677+
alreadyCopiedSourceDocIds: new Set(),
1678+
now,
1679+
})
1680+
1681+
// The tx mock returns its rows regardless of the predicate, so the exclusion is only
1682+
// observable in the condition tree.
1683+
expect(
1684+
flattenMockConditions(wheres[0]).some(
1685+
(node) => node.type === 'isNull' && node.column === schemaMock.document.connectorId
1686+
)
1687+
).toBe(true)
1688+
})
1689+
15871690
it('skips a referenced doc whose parent KB is not mapped (reference is left to be cleared)', async () => {
15881691
const { tx, inserted } = makeTx([sourceRow('doc-1', 'unmapped-kb')])
15891692
const result = await planForkMappedKbDocumentCopies({

apps/sim/ee/workspace-forking/lib/copy/copy-resources.ts

Lines changed: 63 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,11 @@ type SkillSkeletonInsert = Omit<typeof skill.$inferInsert, 'content'> & { conten
368368
* {@link copyForkResourceContent} to copy best-effort after commit. Secrets are
369369
* never copied: MCP OAuth tokens are omitted (re-auth required) and KB connectors
370370
* are not copied (the child is a content snapshot without live sync).
371+
*
372+
* Because the child gets no connector, connector-MANAGED documents are not copied
373+
* either - only hand-uploaded ones. A detached copy is unreachable by the sync engine
374+
* (which keys off `connector_id`), so re-attaching a connector in the child would layer
375+
* a fresh generation on top of it instead of updating it. See {@link copyForkResourceContent}.
371376
*/
372377
export async function copyForkResourceContainers(
373378
params: CopyResourcesParams
@@ -784,6 +789,12 @@ export async function copyForkResourceContainers(
784789
* Each deterministic placeholder is archived with no storage key and zero bytes, so it is
785790
* non-billable until {@link copyForkResourceContent} activates it atomically with accounting.
786791
* Documents whose parent KB is not copied are skipped, leaving their references to be cleared.
792+
*
793+
* Connector-managed documents are skipped for the same reason {@link copyForkResourceContent}
794+
* excludes them from the bulk copy - a detached snapshot the child's connector would duplicate.
795+
* Skipping them HERE too is what keeps the two sides consistent: a placeholder with no content
796+
* phase behind it would stay archived forever while its persisted `knowledge_document` mapping
797+
* pointed at it. Their references clear like any other uncopied document's.
787798
*/
788799
async function createForkDocumentPlaceholders(params: {
789800
tx: DbOrTx
@@ -803,6 +814,7 @@ async function createForkDocumentPlaceholders(params: {
803814
and(
804815
inArray(document.id, referencedDocumentIds),
805816
inArray(document.knowledgeBaseId, Array.from(kbIdMap.keys())),
817+
isNull(document.connectorId),
806818
isNull(document.deletedAt),
807819
isNull(document.archivedAt)
808820
)
@@ -845,7 +857,9 @@ async function createForkDocumentPlaceholders(params: {
845857
* Documents whose parent KB is being copied THIS sync are handled by
846858
* {@link createForkDocumentPlaceholders} under that copied KB and are excluded here via
847859
* `alreadyCopiedSourceDocIds`. A referenced document whose parent KB is not mapped at all is left
848-
* untouched, so its reference is cleared as before.
860+
* untouched, so its reference is cleared as before. Connector-managed documents are excluded for
861+
* the reason given on {@link copyForkResourceContent} - and the exclusion matters MORE here, since
862+
* the target KB is an existing one that may already run its own connector over the same source.
849863
*/
850864
export async function planForkMappedKbDocumentCopies(params: {
851865
tx: DbOrTx
@@ -877,6 +891,7 @@ export async function planForkMappedKbDocumentCopies(params: {
877891
.where(
878892
and(
879893
inArray(document.id, candidateIds),
894+
isNull(document.connectorId),
880895
isNull(document.deletedAt),
881896
isNull(document.archivedAt)
882897
)
@@ -1124,13 +1139,51 @@ export async function copyForkResourceContent(params: {
11241139

11251140
for (const kb of contentPlan.knowledgeBases) {
11261141
try {
1142+
// Connector-managed documents are excluded from the copy (see the predicate below), and a
1143+
// KB can be entirely connector-sourced - so report what was left behind rather than letting
1144+
// the child silently land with fewer documents than the source.
1145+
const [{ skipped: connectorManaged = 0 } = {}] = await db
1146+
.select({ skipped: sql<number>`count(*)` })
1147+
.from(document)
1148+
.where(
1149+
and(
1150+
eq(document.knowledgeBaseId, kb.sourceId),
1151+
isNotNull(document.connectorId),
1152+
isNull(document.deletedAt),
1153+
isNull(document.archivedAt)
1154+
)
1155+
)
1156+
if (Number(connectorManaged) > 0) {
1157+
logger.info(
1158+
`[${requestId}] Skipped connector-managed documents in a copied knowledge base`,
1159+
{
1160+
sourceKnowledgeBaseId: kb.sourceId,
1161+
childKnowledgeBaseId: kb.childId,
1162+
skipped: Number(connectorManaged),
1163+
}
1164+
)
1165+
}
11271166
let afterDocId: string | null = null
11281167
for (;;) {
11291168
// Only copy LIVE documents - exclude soft-deleted and archived rows, matching
11301169
// how the rest of the KB system treats them as gone (chunks/tags/search filter
11311170
// both). A fork must not resurrect documents removed from the source base.
1171+
//
1172+
// Connector-managed documents (`connector_id IS NOT NULL`) are excluded too. A copy can
1173+
// only be a DETACHED snapshot - the child gets no connector (see
1174+
// `copyForkResourceContainers`), and the sync engine keys every existing/tombstone/
1175+
// exclusion lookup off `connector_id`, so a detached copy is invisible to it: it can
1176+
// never be updated, reconciled, or purged, and `doc_connector_external_id_idx`
1177+
// (UNIQUE on `(connector_id, external_id)`) does not constrain it because its
1178+
// `connector_id` is NULL. Attaching a connector to the child then re-ingests every
1179+
// page as a NEW row on top of the snapshot, stacking one duplicate generation per fork
1180+
// hop and returning the same page several times from one retrieval. Skipping them
1181+
// instead leaves the child's connector as the single owner of that content. A source
1182+
// document whose connector was DELETED already has a null `connector_id` (the FK is
1183+
// ON DELETE SET NULL) and is static content in the source too, so it still copies.
11321184
const liveDocs = and(
11331185
eq(document.knowledgeBaseId, kb.sourceId),
1186+
isNull(document.connectorId),
11341187
isNull(document.deletedAt),
11351188
isNull(document.archivedAt)
11361189
)
@@ -1273,6 +1326,15 @@ export async function copyForkResourceContent(params: {
12731326
if (!source) {
12741327
throw new Error(`Source document ${docEntry.sourceDocId} is missing`)
12751328
}
1329+
if (source.connectorId) {
1330+
// Only reachable from a payload planned before connector-managed documents were excluded
1331+
// (a rolling deploy). Fail the entry instead of filling it: the per-document cleanup
1332+
// below drops the archived placeholder and clears its references, which is the outcome
1333+
// the planner would now produce anyway.
1334+
throw new Error(
1335+
`Source document ${docEntry.sourceDocId} is connector-managed and is not copied across a fork edge`
1336+
)
1337+
}
12761338
const resolvedBillingContext = await getBillingContext()
12771339
await copyKbDocument({
12781340
source,

0 commit comments

Comments
 (0)