Skip to content

Commit 0a63dda

Browse files
fix(knowledge): backfill legacy KB workspace ownership (#7600)
* fix(knowledge): backfill legacy KB workspace ownership * fix(db): honor migration runner lock wait policy
1 parent 185e24d commit 0a63dda

17 files changed

Lines changed: 1092 additions & 140 deletions

apps/sim/hooks/queries/kb/knowledge.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
searchWorkspaceKnowledgeContract,
4545
type TagDefinitionData,
4646
type TagUsageData,
47+
type UpdateKnowledgeBaseBody,
4748
type UpdateKnowledgeDocumentResponseData,
4849
updateKnowledgeBaseContract,
4950
updateKnowledgeChunkContract,
@@ -671,13 +672,7 @@ export function useCreateKnowledgeBase() {
671672

672673
interface UpdateKnowledgeBaseParams {
673674
knowledgeBaseId: string
674-
updates: {
675-
name?: string
676-
description?: string
677-
workspaceId?: string | null
678-
/** Moves the knowledge base between folders; `null` moves it to the workspace root. */
679-
folderId?: string | null
680-
}
675+
updates: UpdateKnowledgeBaseBody
681676
}
682677

683678
async function updateKnowledgeBase({

apps/sim/lib/api/contracts/knowledge/base.test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,34 @@ import {
66
chunkingStrategyOptionsSchema,
77
createKnowledgeBaseBodySchema,
88
knowledgeBaseDataSchema,
9+
updateKnowledgeBaseBodySchema,
910
} from '@/lib/api/contracts/knowledge/base'
1011
import { MAX_CHUNKING_SEPARATOR_LENGTH, MAX_CHUNKING_SEPARATORS } from '@/lib/chunkers/constants'
1112

1213
const separators = (count: number) => Array.from({ length: count }, (_, i) => `@@sep${i}@@`)
1314

15+
describe('knowledge base workspace ownership', () => {
16+
it.each([undefined, null, ''])('rejects creation with workspaceId %s', (workspaceId) => {
17+
expect(createKnowledgeBaseBodySchema.safeParse({ name: 'Docs', workspaceId }).success).toBe(
18+
false
19+
)
20+
})
21+
22+
it.each([null, ''])('rejects detaching with workspaceId %s', (workspaceId) => {
23+
expect(updateKnowledgeBaseBodySchema.safeParse({ workspaceId }).success).toBe(false)
24+
})
25+
26+
it('allows a workspace move and moving a KB to the folder root', () => {
27+
expect(
28+
updateKnowledgeBaseBodySchema.parse({ workspaceId: 'workspace-2', folderId: null })
29+
).toEqual({ workspaceId: 'workspace-2', folderId: null })
30+
})
31+
32+
it('leaves workspace ownership untouched when omitted from an update', () => {
33+
expect(updateKnowledgeBaseBodySchema.parse({ name: 'Renamed' })).toEqual({ name: 'Renamed' })
34+
})
35+
})
36+
1437
describe('chunkingStrategyOptionsSchema.separators', () => {
1538
it('accepts a separator list at the bound', () => {
1639
const parsed = chunkingStrategyOptionsSchema.parse({

apps/sim/lib/api/contracts/knowledge/base.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export const createKnowledgeBaseBodySchema = z.object({
148148
`Description must be ${KNOWLEDGE_BASE_DESCRIPTION_MAX_LENGTH} characters or less`
149149
)
150150
.optional(),
151-
workspaceId: z.string().min(1, 'Workspace ID is required'),
151+
workspaceId: workspaceIdSchema,
152152
/**
153153
* Folder the knowledge base is created in, from the `knowledge_base` folder tree.
154154
* `null` (or omitted) creates it at the workspace root.
@@ -170,9 +170,11 @@ export const updateKnowledgeBaseBodySchema = createKnowledgeBaseBodySchema
170170
* explicit `null` moves it back to the workspace root.
171171
*/
172172
folderId: z.string().min(1, 'Folder ID cannot be empty').nullable().optional(),
173-
workspaceId: z.string().nullable().optional(),
173+
workspaceId: workspaceIdSchema.optional(),
174174
})
175175

176+
export type UpdateKnowledgeBaseBody = z.input<typeof updateKnowledgeBaseBodySchema>
177+
176178
const knowledgeChunkingConfigSchema = z
177179
.object({
178180
maxSize: z.number(),

apps/sim/lib/knowledge/__integration__/search-index-policy.integration.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,11 +274,12 @@ describe('canonical search knowledge-base policy', () => {
274274
})
275275
).rejects.toThrow('Only workspace admins')
276276
await expect(
277+
/** @ts-expect-error Exercise a caller bypassing the HTTP contract. */
277278
updateKnowledgeBase(ids.knowledgeBaseId, { workspaceId: null }, 'fixture-detach', {
278279
assertedWorkspaceId: ids.workspaceId,
279280
actorUserId: ids.bobId,
280281
})
281-
).rejects.toThrow('The search index must stay in its workspace')
282+
).rejects.toThrow('Workspace ID is required')
282283
await expectIndexActive()
283284
})
284285

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
/** Real PostgreSQL coverage for workspace deletion and knowledge base ownership. */
2+
import { db } from '@sim/db'
3+
import {
4+
document,
5+
knowledgeBase,
6+
knowledgeConnector,
7+
organization,
8+
user,
9+
workspace,
10+
} from '@sim/db/schema'
11+
import { generateId } from '@sim/utils/id'
12+
import { eq, inArray } from 'drizzle-orm'
13+
import { afterEach, describe, expect, it, vi } from 'vitest'
14+
15+
vi.mock('@/lib/mcp/pubsub', () => ({ mcpPubSub: null }))
16+
vi.mock('@/lib/mcp/service', () => ({
17+
mcpService: { clearCache: vi.fn().mockResolvedValue(undefined) },
18+
}))
19+
vi.mock('@/lib/workflows/lifecycle', () => ({
20+
archiveWorkflowsForWorkspace: vi.fn().mockResolvedValue(0),
21+
}))
22+
23+
import {
24+
createKnowledgeAclFixtureIds,
25+
seedKnowledgeAclFixture,
26+
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
27+
import {
28+
createAuthorizedKnowledgeBase,
29+
restoreKnowledgeBase,
30+
updateKnowledgeBase,
31+
} from '@/lib/knowledge/service'
32+
import { archiveWorkspace } from '@/lib/workspaces/lifecycle'
33+
34+
describe('workspace knowledge lifecycle in PostgreSQL', () => {
35+
const fixtures: ReturnType<typeof createKnowledgeAclFixtureIds>[] = []
36+
37+
async function seed() {
38+
const ids = createKnowledgeAclFixtureIds()
39+
fixtures.push(ids)
40+
await seedKnowledgeAclFixture(ids)
41+
const documentId = generateId()
42+
await db.insert(document).values({
43+
id: documentId,
44+
knowledgeBaseId: ids.knowledgeBaseId,
45+
filename: 'fixture.txt',
46+
fileUrl: 'data:text/plain,fixture',
47+
fileSize: 7,
48+
mimeType: 'text/plain',
49+
processingStatus: 'completed',
50+
})
51+
return { ...ids, documentId }
52+
}
53+
54+
afterEach(async () => {
55+
for (const ids of fixtures.splice(0)) {
56+
await db.delete(workspace).where(eq(workspace.id, ids.workspaceId))
57+
await db.delete(organization).where(eq(organization.id, ids.organizationId))
58+
await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId]))
59+
}
60+
})
61+
62+
it.each([false, true])(
63+
'deletes child KBs when workspace was already archived: %s',
64+
async (alreadyArchived) => {
65+
const ids = await seed()
66+
const other = await seed()
67+
const previousArchive = new Date('2025-01-01T00:00:00.000Z')
68+
if (alreadyArchived) {
69+
await db
70+
.update(workspace)
71+
.set({ archivedAt: previousArchive })
72+
.where(eq(workspace.id, ids.workspaceId))
73+
}
74+
75+
expect(
76+
await archiveWorkspace(ids.workspaceId, { requestId: 'lifecycle-fixture' })
77+
).toMatchObject({
78+
archived: !alreadyArchived,
79+
})
80+
81+
const [ws] = await db.select().from(workspace).where(eq(workspace.id, ids.workspaceId))
82+
const [kb] = await db
83+
.select()
84+
.from(knowledgeBase)
85+
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
86+
const [doc] = await db.select().from(document).where(eq(document.id, ids.documentId))
87+
const [connector] = await db
88+
.select()
89+
.from(knowledgeConnector)
90+
.where(eq(knowledgeConnector.id, ids.connectorId))
91+
expect(ws.archivedAt).toBeInstanceOf(Date)
92+
expect(kb).toMatchObject({ workspaceId: ids.workspaceId, deletedAt: ws.archivedAt })
93+
expect(doc).toMatchObject({ archivedAt: ws.archivedAt, deletedAt: null })
94+
expect(connector).toMatchObject({ archivedAt: ws.archivedAt, status: 'paused' })
95+
if (alreadyArchived) expect(ws.archivedAt).toEqual(previousArchive)
96+
97+
await expect(
98+
restoreKnowledgeBase(ids.knowledgeBaseId, 'lifecycle-fixture')
99+
).rejects.toMatchObject({
100+
code: 'conflict',
101+
message: 'Cannot restore knowledge base into an archived workspace',
102+
})
103+
await archiveWorkspace(ids.workspaceId, { requestId: 'lifecycle-retry' })
104+
const [unchanged] = await db
105+
.select()
106+
.from(knowledgeBase)
107+
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
108+
expect(unchanged.deletedAt).toEqual(kb.deletedAt)
109+
const [otherKb] = await db
110+
.select()
111+
.from(knowledgeBase)
112+
.where(eq(knowledgeBase.id, other.knowledgeBaseId))
113+
expect(otherKb.deletedAt).toBeNull()
114+
const [otherDoc] = await db.select().from(document).where(eq(document.id, other.documentId))
115+
expect(otherDoc.archivedAt).toBeNull()
116+
}
117+
)
118+
119+
it('hard deletion cascades to KBs, documents, and connectors', async () => {
120+
const ids = await seed()
121+
await db.delete(workspace).where(eq(workspace.id, ids.workspaceId))
122+
123+
expect(
124+
await db
125+
.select({ id: knowledgeBase.id })
126+
.from(knowledgeBase)
127+
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
128+
).toEqual([])
129+
expect(
130+
await db.select({ id: document.id }).from(document).where(eq(document.id, ids.documentId))
131+
).toEqual([])
132+
expect(
133+
await db
134+
.select({ id: knowledgeConnector.id })
135+
.from(knowledgeConnector)
136+
.where(eq(knowledgeConnector.id, ids.connectorId))
137+
).toEqual([])
138+
})
139+
140+
it('refuses owner detachment without changing the KB or its documents', async () => {
141+
const ids = await seed()
142+
await expect(
143+
/** @ts-expect-error Exercise a caller bypassing the HTTP contract. */
144+
updateKnowledgeBase(ids.knowledgeBaseId, { workspaceId: null }, 'lifecycle-fixture', {
145+
actorUserId: ids.aliceId,
146+
})
147+
).rejects.toMatchObject({ code: 'validation' })
148+
const [kb] = await db
149+
.select()
150+
.from(knowledgeBase)
151+
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
152+
expect(kb).toMatchObject({ workspaceId: ids.workspaceId, deletedAt: null })
153+
const [doc] = await db.select().from(document).where(eq(document.id, ids.documentId))
154+
expect(doc.archivedAt).toBeNull()
155+
})
156+
157+
it('requires an owner for creation while allowing organization-owned indexes', async () => {
158+
const ids = await seed()
159+
const data = {
160+
name: 'Organization search',
161+
userId: ids.aliceId,
162+
embeddingModel: 'text-embedding-3-small',
163+
embeddingDimension: 1536 as const,
164+
chunkingConfig: { maxSize: 1024, minSize: 1, overlap: 20 },
165+
isSearchIndex: true,
166+
}
167+
await expect(createAuthorizedKnowledgeBase(data, 'lifecycle-fixture')).rejects.toThrow(
168+
'Resource requires exactly one workspace or organization owner'
169+
)
170+
const kb = await createAuthorizedKnowledgeBase(
171+
{ ...data, organizationId: ids.organizationId },
172+
'lifecycle-fixture'
173+
)
174+
expect(kb).toMatchObject({ workspaceId: null, organizationId: ids.organizationId })
175+
})
176+
})

apps/sim/lib/knowledge/application/knowledge-bases.test.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -517,6 +517,51 @@ describe('knowledge base application use cases', () => {
517517
)
518518
})
519519

520+
it.each([null, ''])('rejects detaching a KB even for its owner: %s', async (workspaceId) => {
521+
await expect(
522+
updateInternalKnowledgeBase.execute({
523+
principal: { kind: 'session', userId: knowledgeBase.userId, sessionId: 'session-1' },
524+
/** @ts-expect-error Exercise a runtime caller bypassing the HTTP contract. */
525+
input: { knowledgeBaseId: knowledgeBase.id, workspaceId },
526+
})
527+
).rejects.toMatchObject({ code: 'validation', message: 'Workspace ID is required' })
528+
expect(mocks.performUpdate).not.toHaveBeenCalled()
529+
expect(mocks.resolveWorkspace).not.toHaveBeenCalled()
530+
expect(mocks.recordAudit).not.toHaveBeenCalled()
531+
})
532+
533+
it('allows a legacy KB owner to move it into an authorized workspace', async () => {
534+
mocks.getRecord.mockResolvedValueOnce({ ...knowledgeBase, workspaceId: null })
535+
536+
await updateInternalKnowledgeBase.execute({
537+
principal: { kind: 'session', userId: knowledgeBase.userId, sessionId: 'session-1' },
538+
input: { knowledgeBaseId: knowledgeBase.id, workspaceId: 'workspace-1' },
539+
})
540+
541+
expect(mocks.resolveWorkspace).toHaveBeenCalledWith({ workspaceId: 'workspace-1' })
542+
expect(mocks.resolvePermission).toHaveBeenCalledTimes(1)
543+
expect(mocks.performUpdate).toHaveBeenCalledWith(
544+
expect.objectContaining({
545+
workspaceId: null,
546+
updates: expect.objectContaining({ workspaceId: 'workspace-1' }),
547+
})
548+
)
549+
})
550+
551+
it('allows metadata edits on a legacy KB without detaching another KB', async () => {
552+
mocks.getRecord.mockResolvedValueOnce({ ...knowledgeBase, workspaceId: null })
553+
554+
await updateInternalKnowledgeBase.execute({
555+
principal: { kind: 'session', userId: knowledgeBase.userId, sessionId: 'session-1' },
556+
input: { knowledgeBaseId: knowledgeBase.id, name: 'Renamed' },
557+
})
558+
559+
expect(mocks.performUpdate).toHaveBeenCalledWith(
560+
expect.objectContaining({ updates: expect.objectContaining({ workspaceId: undefined }) })
561+
)
562+
expect(mocks.resolveWorkspace).not.toHaveBeenCalled()
563+
})
564+
520565
it('rejects a destination workspace before an internal move mutation', async () => {
521566
mocks.resolveWorkspace.mockResolvedValueOnce({ ...context, workspaceId: 'workspace-2' })
522567
mocks.resolvePermission.mockResolvedValueOnce('write').mockResolvedValueOnce(null)

apps/sim/lib/knowledge/application/knowledge-bases.ts

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -200,7 +200,7 @@ export interface ReadInternalKnowledgeBaseInput {
200200
export interface UpdateInternalKnowledgeBaseInput extends ReadInternalKnowledgeBaseInput {
201201
name?: string
202202
description?: string
203-
workspaceId?: string | null
203+
workspaceId?: string
204204
folderId?: string | null
205205
chunkingConfig?: ChunkingConfig
206206
}
@@ -777,20 +777,15 @@ export const updateInternalKnowledgeBase = {
777777
const knowledgeBase = await loadInternalActiveKnowledgeBase(input.knowledgeBaseId)
778778
await authorizeInternalKnowledgeBase(principal, knowledgeBase, knowledgeOperations.update)
779779

780+
if (input.workspaceId !== undefined && !input.workspaceId) {
781+
throw new OrchestrationError('validation', 'Workspace ID is required')
782+
}
783+
780784
if (input.workspaceId !== undefined && input.workspaceId !== knowledgeBase.workspaceId) {
781-
if (input.workspaceId === null) {
782-
if (knowledgeBase.userId !== principal.userId) {
783-
throw new OrchestrationError(
784-
'forbidden',
785-
'Only the knowledge base owner can remove it from a workspace'
786-
)
787-
}
788-
} else {
789-
const destination = await resolveKnowledgeWorkspaceContext({
790-
workspaceId: input.workspaceId,
791-
})
792-
await authorizeWorkspaceOperation(principal, knowledgeOperations.update, destination)
793-
}
785+
const destination = await resolveKnowledgeWorkspaceContext({
786+
workspaceId: input.workspaceId,
787+
})
788+
await authorizeWorkspaceOperation(principal, knowledgeOperations.update, destination)
794789
}
795790

796791
const outcome = await performUpdateKnowledgeBase({

apps/sim/lib/knowledge/orchestration/knowledge-bases.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ export interface PerformUpdateKnowledgeBaseParams extends KnowledgeOperationCont
128128
name?: string
129129
description?: string
130130
/** Moves the knowledge base between workspaces; omitted leaves it in place. */
131-
workspaceId?: string | null
131+
workspaceId?: string
132132
folderId?: string | null
133133
chunkingConfig?: ChunkingConfig
134134
}

0 commit comments

Comments
 (0)