Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions apps/sim/hooks/queries/kb/knowledge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ import {
searchWorkspaceKnowledgeContract,
type TagDefinitionData,
type TagUsageData,
type UpdateKnowledgeBaseBody,
type UpdateKnowledgeDocumentResponseData,
updateKnowledgeBaseContract,
updateKnowledgeChunkContract,
Expand Down Expand Up @@ -671,13 +672,7 @@ export function useCreateKnowledgeBase() {

interface UpdateKnowledgeBaseParams {
knowledgeBaseId: string
updates: {
name?: string
description?: string
workspaceId?: string | null
/** Moves the knowledge base between folders; `null` moves it to the workspace root. */
folderId?: string | null
}
updates: UpdateKnowledgeBaseBody
}

async function updateKnowledgeBase({
Expand Down
23 changes: 23 additions & 0 deletions apps/sim/lib/api/contracts/knowledge/base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,34 @@ import {
chunkingStrategyOptionsSchema,
createKnowledgeBaseBodySchema,
knowledgeBaseDataSchema,
updateKnowledgeBaseBodySchema,
} from '@/lib/api/contracts/knowledge/base'
import { MAX_CHUNKING_SEPARATOR_LENGTH, MAX_CHUNKING_SEPARATORS } from '@/lib/chunkers/constants'

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

describe('knowledge base workspace ownership', () => {
it.each([undefined, null, ''])('rejects creation with workspaceId %s', (workspaceId) => {
expect(createKnowledgeBaseBodySchema.safeParse({ name: 'Docs', workspaceId }).success).toBe(
false
)
})

it.each([null, ''])('rejects detaching with workspaceId %s', (workspaceId) => {
expect(updateKnowledgeBaseBodySchema.safeParse({ workspaceId }).success).toBe(false)
})

it('allows a workspace move and moving a KB to the folder root', () => {
expect(
updateKnowledgeBaseBodySchema.parse({ workspaceId: 'workspace-2', folderId: null })
).toEqual({ workspaceId: 'workspace-2', folderId: null })
})

it('leaves workspace ownership untouched when omitted from an update', () => {
expect(updateKnowledgeBaseBodySchema.parse({ name: 'Renamed' })).toEqual({ name: 'Renamed' })
})
})

describe('chunkingStrategyOptionsSchema.separators', () => {
it('accepts a separator list at the bound', () => {
const parsed = chunkingStrategyOptionsSchema.parse({
Expand Down
6 changes: 4 additions & 2 deletions apps/sim/lib/api/contracts/knowledge/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export const createKnowledgeBaseBodySchema = z.object({
`Description must be ${KNOWLEDGE_BASE_DESCRIPTION_MAX_LENGTH} characters or less`
)
.optional(),
workspaceId: z.string().min(1, 'Workspace ID is required'),
workspaceId: workspaceIdSchema,
/**
* Folder the knowledge base is created in, from the `knowledge_base` folder tree.
* `null` (or omitted) creates it at the workspace root.
Expand All @@ -170,9 +170,11 @@ export const updateKnowledgeBaseBodySchema = createKnowledgeBaseBodySchema
* explicit `null` moves it back to the workspace root.
*/
folderId: z.string().min(1, 'Folder ID cannot be empty').nullable().optional(),
workspaceId: z.string().nullable().optional(),
workspaceId: workspaceIdSchema.optional(),
})

export type UpdateKnowledgeBaseBody = z.input<typeof updateKnowledgeBaseBodySchema>

const knowledgeChunkingConfigSchema = z
.object({
maxSize: z.number(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,11 +274,12 @@ describe('canonical search knowledge-base policy', () => {
})
).rejects.toThrow('Only workspace admins')
await expect(
/** @ts-expect-error Exercise a caller bypassing the HTTP contract. */
updateKnowledgeBase(ids.knowledgeBaseId, { workspaceId: null }, 'fixture-detach', {
assertedWorkspaceId: ids.workspaceId,
actorUserId: ids.bobId,
})
).rejects.toThrow('The search index must stay in its workspace')
).rejects.toThrow('Workspace ID is required')
await expectIndexActive()
})

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
/** Real PostgreSQL coverage for workspace deletion and knowledge base ownership. */
import { db } from '@sim/db'
import {
document,
knowledgeBase,
knowledgeConnector,
organization,
user,
workspace,
} from '@sim/db/schema'
import { generateId } from '@sim/utils/id'
import { eq, inArray } from 'drizzle-orm'
import { afterEach, describe, expect, it, vi } from 'vitest'

vi.mock('@/lib/mcp/pubsub', () => ({ mcpPubSub: null }))
vi.mock('@/lib/mcp/service', () => ({
mcpService: { clearCache: vi.fn().mockResolvedValue(undefined) },
}))
vi.mock('@/lib/workflows/lifecycle', () => ({
archiveWorkflowsForWorkspace: vi.fn().mockResolvedValue(0),
}))

import {
createKnowledgeAclFixtureIds,
seedKnowledgeAclFixture,
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
import {
createAuthorizedKnowledgeBase,
restoreKnowledgeBase,
updateKnowledgeBase,
} from '@/lib/knowledge/service'
import { archiveWorkspace } from '@/lib/workspaces/lifecycle'

describe('workspace knowledge lifecycle in PostgreSQL', () => {
const fixtures: ReturnType<typeof createKnowledgeAclFixtureIds>[] = []

async function seed() {
const ids = createKnowledgeAclFixtureIds()
fixtures.push(ids)
await seedKnowledgeAclFixture(ids)
const documentId = generateId()
await db.insert(document).values({
id: documentId,
knowledgeBaseId: ids.knowledgeBaseId,
filename: 'fixture.txt',
fileUrl: 'data:text/plain,fixture',
fileSize: 7,
mimeType: 'text/plain',
processingStatus: 'completed',
})
return { ...ids, documentId }
}

afterEach(async () => {
for (const ids of fixtures.splice(0)) {
await db.delete(workspace).where(eq(workspace.id, ids.workspaceId))
await db.delete(organization).where(eq(organization.id, ids.organizationId))
await db.delete(user).where(inArray(user.id, [ids.aliceId, ids.bobId]))
}
})

it.each([false, true])(
'deletes child KBs when workspace was already archived: %s',
async (alreadyArchived) => {
const ids = await seed()
const other = await seed()
const previousArchive = new Date('2025-01-01T00:00:00.000Z')
if (alreadyArchived) {
await db
.update(workspace)
.set({ archivedAt: previousArchive })
.where(eq(workspace.id, ids.workspaceId))
}

expect(
await archiveWorkspace(ids.workspaceId, { requestId: 'lifecycle-fixture' })
).toMatchObject({
archived: !alreadyArchived,
})

const [ws] = await db.select().from(workspace).where(eq(workspace.id, ids.workspaceId))
const [kb] = await db
.select()
.from(knowledgeBase)
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
const [doc] = await db.select().from(document).where(eq(document.id, ids.documentId))
const [connector] = await db
.select()
.from(knowledgeConnector)
.where(eq(knowledgeConnector.id, ids.connectorId))
expect(ws.archivedAt).toBeInstanceOf(Date)
expect(kb).toMatchObject({ workspaceId: ids.workspaceId, deletedAt: ws.archivedAt })
expect(doc).toMatchObject({ archivedAt: ws.archivedAt, deletedAt: null })
expect(connector).toMatchObject({ archivedAt: ws.archivedAt, status: 'paused' })
if (alreadyArchived) expect(ws.archivedAt).toEqual(previousArchive)

await expect(
restoreKnowledgeBase(ids.knowledgeBaseId, 'lifecycle-fixture')
).rejects.toMatchObject({
code: 'conflict',
message: 'Cannot restore knowledge base into an archived workspace',
})
await archiveWorkspace(ids.workspaceId, { requestId: 'lifecycle-retry' })
const [unchanged] = await db
.select()
.from(knowledgeBase)
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
expect(unchanged.deletedAt).toEqual(kb.deletedAt)
const [otherKb] = await db
.select()
.from(knowledgeBase)
.where(eq(knowledgeBase.id, other.knowledgeBaseId))
expect(otherKb.deletedAt).toBeNull()
const [otherDoc] = await db.select().from(document).where(eq(document.id, other.documentId))
expect(otherDoc.archivedAt).toBeNull()
}
)

it('hard deletion cascades to KBs, documents, and connectors', async () => {
const ids = await seed()
await db.delete(workspace).where(eq(workspace.id, ids.workspaceId))

expect(
await db
.select({ id: knowledgeBase.id })
.from(knowledgeBase)
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
).toEqual([])
expect(
await db.select({ id: document.id }).from(document).where(eq(document.id, ids.documentId))
).toEqual([])
expect(
await db
.select({ id: knowledgeConnector.id })
.from(knowledgeConnector)
.where(eq(knowledgeConnector.id, ids.connectorId))
).toEqual([])
})

it('refuses owner detachment without changing the KB or its documents', async () => {
const ids = await seed()
await expect(
/** @ts-expect-error Exercise a caller bypassing the HTTP contract. */
updateKnowledgeBase(ids.knowledgeBaseId, { workspaceId: null }, 'lifecycle-fixture', {
actorUserId: ids.aliceId,
})
).rejects.toMatchObject({ code: 'validation' })
const [kb] = await db
.select()
.from(knowledgeBase)
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
expect(kb).toMatchObject({ workspaceId: ids.workspaceId, deletedAt: null })
const [doc] = await db.select().from(document).where(eq(document.id, ids.documentId))
expect(doc.archivedAt).toBeNull()
})

it('requires an owner for creation while allowing organization-owned indexes', async () => {
const ids = await seed()
const data = {
name: 'Organization search',
userId: ids.aliceId,
embeddingModel: 'text-embedding-3-small',
embeddingDimension: 1536 as const,
chunkingConfig: { maxSize: 1024, minSize: 1, overlap: 20 },
isSearchIndex: true,
}
await expect(createAuthorizedKnowledgeBase(data, 'lifecycle-fixture')).rejects.toThrow(
'Resource requires exactly one workspace or organization owner'
)
const kb = await createAuthorizedKnowledgeBase(
{ ...data, organizationId: ids.organizationId },
'lifecycle-fixture'
)
expect(kb).toMatchObject({ workspaceId: null, organizationId: ids.organizationId })
})
})
45 changes: 45 additions & 0 deletions apps/sim/lib/knowledge/application/knowledge-bases.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,51 @@ describe('knowledge base application use cases', () => {
)
})

it.each([null, ''])('rejects detaching a KB even for its owner: %s', async (workspaceId) => {
await expect(
updateInternalKnowledgeBase.execute({
principal: { kind: 'session', userId: knowledgeBase.userId, sessionId: 'session-1' },
/** @ts-expect-error Exercise a runtime caller bypassing the HTTP contract. */
input: { knowledgeBaseId: knowledgeBase.id, workspaceId },
})
).rejects.toMatchObject({ code: 'validation', message: 'Workspace ID is required' })
expect(mocks.performUpdate).not.toHaveBeenCalled()
expect(mocks.resolveWorkspace).not.toHaveBeenCalled()
expect(mocks.recordAudit).not.toHaveBeenCalled()
})

it('allows a legacy KB owner to move it into an authorized workspace', async () => {
mocks.getRecord.mockResolvedValueOnce({ ...knowledgeBase, workspaceId: null })

await updateInternalKnowledgeBase.execute({
principal: { kind: 'session', userId: knowledgeBase.userId, sessionId: 'session-1' },
input: { knowledgeBaseId: knowledgeBase.id, workspaceId: 'workspace-1' },
})

expect(mocks.resolveWorkspace).toHaveBeenCalledWith({ workspaceId: 'workspace-1' })
expect(mocks.resolvePermission).toHaveBeenCalledTimes(1)
expect(mocks.performUpdate).toHaveBeenCalledWith(
expect.objectContaining({
workspaceId: null,
updates: expect.objectContaining({ workspaceId: 'workspace-1' }),
})
)
})

it('allows metadata edits on a legacy KB without detaching another KB', async () => {
mocks.getRecord.mockResolvedValueOnce({ ...knowledgeBase, workspaceId: null })

await updateInternalKnowledgeBase.execute({
principal: { kind: 'session', userId: knowledgeBase.userId, sessionId: 'session-1' },
input: { knowledgeBaseId: knowledgeBase.id, name: 'Renamed' },
})

expect(mocks.performUpdate).toHaveBeenCalledWith(
expect.objectContaining({ updates: expect.objectContaining({ workspaceId: undefined }) })
)
expect(mocks.resolveWorkspace).not.toHaveBeenCalled()
})

it('rejects a destination workspace before an internal move mutation', async () => {
mocks.resolveWorkspace.mockResolvedValueOnce({ ...context, workspaceId: 'workspace-2' })
mocks.resolvePermission.mockResolvedValueOnce('write').mockResolvedValueOnce(null)
Expand Down
23 changes: 9 additions & 14 deletions apps/sim/lib/knowledge/application/knowledge-bases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ export interface ReadInternalKnowledgeBaseInput {
export interface UpdateInternalKnowledgeBaseInput extends ReadInternalKnowledgeBaseInput {
name?: string
description?: string
workspaceId?: string | null
workspaceId?: string
folderId?: string | null
chunkingConfig?: ChunkingConfig
}
Expand Down Expand Up @@ -777,20 +777,15 @@ export const updateInternalKnowledgeBase = {
const knowledgeBase = await loadInternalActiveKnowledgeBase(input.knowledgeBaseId)
await authorizeInternalKnowledgeBase(principal, knowledgeBase, knowledgeOperations.update)

if (input.workspaceId !== undefined && !input.workspaceId) {
throw new OrchestrationError('validation', 'Workspace ID is required')
}

if (input.workspaceId !== undefined && input.workspaceId !== knowledgeBase.workspaceId) {
if (input.workspaceId === null) {
if (knowledgeBase.userId !== principal.userId) {
throw new OrchestrationError(
'forbidden',
'Only the knowledge base owner can remove it from a workspace'
)
}
} else {
const destination = await resolveKnowledgeWorkspaceContext({
workspaceId: input.workspaceId,
})
await authorizeWorkspaceOperation(principal, knowledgeOperations.update, destination)
}
const destination = await resolveKnowledgeWorkspaceContext({
workspaceId: input.workspaceId,
})
await authorizeWorkspaceOperation(principal, knowledgeOperations.update, destination)
}

const outcome = await performUpdateKnowledgeBase({
Expand Down
2 changes: 1 addition & 1 deletion apps/sim/lib/knowledge/orchestration/knowledge-bases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ export interface PerformUpdateKnowledgeBaseParams extends KnowledgeOperationCont
name?: string
description?: string
/** Moves the knowledge base between workspaces; omitted leaves it in place. */
workspaceId?: string | null
workspaceId?: string
folderId?: string | null
chunkingConfig?: ChunkingConfig
}
Expand Down
Loading
Loading