Skip to content

Commit 303c84d

Browse files
committed
fix(search-mcp): merge staging and clean up failed chats
2 parents 5d55989 + 5465aed commit 303c84d

40 files changed

Lines changed: 26510 additions & 1255 deletions

apps/sim/app/api/v1/capability-gate.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ vi.mock('@/lib/table', () => ({
7878
}))
7979
vi.mock('@/lib/table/wire', () => ({ normalizeColumn: (column: unknown) => column }))
8080
vi.mock('@/lib/knowledge/service', () => ({
81-
listWorkspaceAndLegacyKnowledgeBases: mockListKnowledgeBases,
81+
getWorkspaceKnowledgeBases: mockListKnowledgeBases,
8282
getKnowledgeBaseById: vi.fn(),
8383
}))
8484
vi.mock('@/lib/knowledge/orchestration', () => ({ performCreateKnowledgeBase: vi.fn() }))
@@ -158,7 +158,7 @@ beforeEach(() => {
158158
mockGetUserEntityPermissions.mockResolvedValue('admin')
159159
mockGetWorkspaceBillingSettings.mockResolvedValue({ allowPersonalApiKeys: true })
160160
mockListTables.mockResolvedValue([])
161-
mockListKnowledgeBases.mockResolvedValue([])
161+
mockListKnowledgeBases.mockResolvedValue({ data: [], nextCursorKeys: null })
162162
mockListWorkspaceFiles.mockResolvedValue([])
163163
mockListPublicWorkflowLogs.mockResolvedValue({ data: [], nextCursor: null })
164164
mockGetDeploymentWorkflowTarget.mockResolvedValue({

apps/sim/app/api/v1/knowledge/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
} from '@/lib/core/orchestration/types'
1111
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
1212
import { performCreateKnowledgeBase } from '@/lib/knowledge/orchestration'
13-
import { listWorkspaceAndLegacyKnowledgeBases } from '@/lib/knowledge/service'
13+
import { getWorkspaceKnowledgeBases } from '@/lib/knowledge/service'
1414
import { formatKnowledgeBase, handleError } from '@/app/api/v1/knowledge/utils'
1515
import {
1616
authenticateRequest,
@@ -50,7 +50,7 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
5050

5151
/** Read only after `validateWorkspaceAccess` authorized this caller; same list the
5252
* internal surface serves, from the same place. */
53-
const knowledgeBases = await listWorkspaceAndLegacyKnowledgeBases(userId, workspaceId)
53+
const { data: knowledgeBases } = await getWorkspaceKnowledgeBases(workspaceId)
5454

5555
return NextResponse.json({
5656
success: true,

apps/sim/lib/knowledge/__integration__/organization-mcp-search.integration.ts

Lines changed: 82 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,9 @@ import {
6868
import { confluencePageAcl } from '@/lib/knowledge/access/confluence-permissions'
6969
import { listKnowledgeChunks } from '@/lib/knowledge/application/chunks'
7070
import { readKnowledgeDocument } from '@/lib/knowledge/application/documents'
71+
import { listKnowledgeBaseCatalog } from '@/lib/knowledge/application/knowledge-bases'
7172
import { readIndexedKnowledgeDocument } from '@/lib/knowledge/application/read-indexed-document'
73+
import { prepareSearchSource } from '@/lib/knowledge/application/sim-search'
7274
import { searchScopedKnowledge } from '@/lib/knowledge/application/workspace-search'
7375
import { createContentSyncLease } from '@/lib/knowledge/connectors/sync-lock'
7476
import { addDocument, persistDocumentAcls } from '@/lib/knowledge/connectors/sync-persistence'
@@ -91,8 +93,9 @@ describe('organization Search MCP with real ingestion and current access', () =>
9193
groupIds,
9294
} = ids
9395
const otherOrganizationId = generateId()
94-
const otherKnowledgeBaseId = generateId()
96+
const workspaceKnowledgeBaseId = generateId()
9597
const outsiderId = generateId()
98+
const otherAdminId = generateId()
9699
const bobMembershipId = generateId()
97100
const tokens = {
98101
alice: generateId(),
@@ -106,10 +109,16 @@ describe('organization Search MCP with real ingestion and current access', () =>
106109
const clients: Client[] = []
107110
const alicePrincipal: Principal = { kind: 'session', userId: aliceId, sessionId: generateId() }
108111
const bobPrincipal: Principal = { kind: 'session', userId: bobId, sessionId: generateId() }
112+
const otherAdminPrincipal: Principal = {
113+
kind: 'session',
114+
userId: otherAdminId,
115+
sessionId: generateId(),
116+
}
109117
const bobSourceMembership = {
110118
groupId: groupIds[2],
111119
subjectToken: `u:${bobId}@fixture.test`,
112120
}
121+
let otherKnowledgeBaseId: string
113122
let documentId: string
114123
let alice: Client
115124
let bob: Client
@@ -233,14 +242,16 @@ describe('organization Search MCP with real ingestion and current access', () =>
233242
})
234243
fixtures.storageRoot = mkdtempSync(path.join(tmpdir(), 'sim-organization-mcp-integration-'))
235244
await seedKnowledgeAclFixture(ids)
236-
await db.insert(user).values({
237-
id: outsiderId,
238-
name: 'Other organization fixture',
239-
email: `${outsiderId}@fixture.test`,
240-
emailVerified: true,
241-
createdAt: new Date(),
242-
updatedAt: new Date(),
243-
})
245+
await db.insert(user).values(
246+
[outsiderId, otherAdminId].map((id) => ({
247+
id,
248+
name: 'Other organization fixture',
249+
email: `${id}@fixture.test`,
250+
emailVerified: true,
251+
createdAt: new Date(),
252+
updatedAt: new Date(),
253+
}))
254+
)
244255
await db.insert(organization).values({
245256
id: otherOrganizationId,
246257
name: 'Other organization MCP fixture',
@@ -251,6 +262,12 @@ describe('organization Search MCP with real ingestion and current access', () =>
251262
{ id: generateId(), userId: aliceId, organizationId, role: 'owner' },
252263
{ id: bobMembershipId, userId: bobId, organizationId, role: 'member' },
253264
{ id: generateId(), userId: outsiderId, organizationId: otherOrganizationId, role: 'owner' },
265+
{
266+
id: generateId(),
267+
userId: otherAdminId,
268+
organizationId: otherOrganizationId,
269+
role: 'admin',
270+
},
254271
])
255272
/** Reuse source identities, but establish exclusive organization ownership before ingestion. */
256273
await db
@@ -267,12 +284,16 @@ describe('organization Search MCP with real ingestion and current access', () =>
267284
.update(knowledgeExternalGroup)
268285
.set({ workspaceId: null, organizationId })
269286
.where(inArray(knowledgeExternalGroup.id, groupIds))
287+
const prepared = await prepareSearchSource.execute({
288+
principal: otherAdminPrincipal,
289+
input: { organizationId: otherOrganizationId, connectorType: 'gitlab' },
290+
})
291+
otherKnowledgeBaseId = prepared.knowledgeBaseId
270292
await db.insert(knowledgeBase).values({
271-
id: otherKnowledgeBaseId,
272-
userId: outsiderId,
273-
organizationId: otherOrganizationId,
274-
isSearchIndex: true,
275-
name: 'Other org index',
293+
id: workspaceKnowledgeBaseId,
294+
userId: bobId,
295+
workspaceId,
296+
name: 'Workspace documents',
276297
})
277298
await db.insert(organizationSearchIntegration).values({
278299
organizationId,
@@ -387,12 +408,56 @@ describe('organization Search MCP with real ingestion and current access', () =>
387408
.delete(organization)
388409
.where(inArray(organization.id, [organizationId, otherOrganizationId]))
389410
await db.delete(workspace).where(eq(workspace.id, workspaceId))
390-
await db.delete(user).where(inArray(user.id, [aliceId, bobId, outsiderId]))
411+
await db.delete(user).where(inArray(user.id, [aliceId, bobId, outsiderId, otherAdminId]))
391412
if (fixtures.storageRoot) await rm(fixtures.storageRoot, { recursive: true, force: true })
392413
await db.$client.end()
393414
vi.unstubAllGlobals()
394415
})
395416

417+
it('creates an organization-only index, keeps it out of the workspace catalog, and separates actor from payer', async () => {
418+
const input = { organizationId: otherOrganizationId, connectorType: 'gitlab' }
419+
const results = await Promise.all([
420+
prepareSearchSource.execute({ principal: otherAdminPrincipal, input }),
421+
prepareSearchSource.execute({ principal: otherAdminPrincipal, input }),
422+
])
423+
expect(results.map((result) => result.knowledgeBaseId)).toEqual([
424+
otherKnowledgeBaseId,
425+
otherKnowledgeBaseId,
426+
])
427+
const indexes = await db
428+
.select()
429+
.from(knowledgeBase)
430+
.where(eq(knowledgeBase.organizationId, otherOrganizationId))
431+
expect(indexes).toEqual([
432+
expect.objectContaining({
433+
id: otherKnowledgeBaseId,
434+
workspaceId: null,
435+
organizationId: otherOrganizationId,
436+
isSearchIndex: true,
437+
userId: otherAdminId,
438+
}),
439+
])
440+
const catalog = await listKnowledgeBaseCatalog.execute({
441+
principal: alicePrincipal,
442+
input: { workspaceId },
443+
})
444+
expect(catalog.knowledgeBases.map(({ knowledgeBase }) => knowledgeBase.id)).toEqual([
445+
workspaceKnowledgeBaseId,
446+
])
447+
await expect(
448+
resolveOrganizationBillingAttribution({
449+
actorUserId: otherAdminId,
450+
organizationId: otherOrganizationId,
451+
})
452+
).resolves.toMatchObject({
453+
actorUserId: otherAdminId,
454+
workspaceId: null,
455+
organizationId: otherOrganizationId,
456+
billedAccountUserId: outsiderId,
457+
billingEntity: { type: 'organization', id: otherOrganizationId },
458+
})
459+
})
460+
396461
it('finds the canonical org-owned index and applies each current member’s source ACL to all tools', async () => {
397462
const [owner] = await db
398463
.select({
@@ -674,7 +739,7 @@ describe('organization Search MCP with real ingestion and current access', () =>
674739
it('returns an actionable empty index without creating one or accepting an alternate knowledge base', async () => {
675740
await db
676741
.update(knowledgeBase)
677-
.set({ isSearchIndex: false })
742+
.set({ deletedAt: new Date() })
678743
.where(eq(knowledgeBase.id, knowledgeBaseId))
679744
try {
680745
const empty = await value(alice, 'search', { query: 'Orion' })
@@ -688,7 +753,7 @@ describe('organization Search MCP with real ingestion and current access', () =>
688753
} finally {
689754
await db
690755
.update(knowledgeBase)
691-
.set({ isSearchIndex: true })
756+
.set({ deletedAt: null })
692757
.where(eq(knowledgeBase.id, knowledgeBaseId))
693758
}
694759
})

apps/sim/lib/knowledge/__integration__/read-indexed-document.integration.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ describe('indexed document references', () => {
195195
it('requires the canonical organization index for both URL and ID reads', async () => {
196196
await db
197197
.update(knowledgeBase)
198-
.set({ isSearchIndex: false })
198+
.set({ deletedAt: new Date() })
199199
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
200200
try {
201201
await expect(read(alice)).rejects.toThrow('Document not found')
@@ -205,7 +205,7 @@ describe('indexed document references', () => {
205205
} finally {
206206
await db
207207
.update(knowledgeBase)
208-
.set({ isSearchIndex: true })
208+
.set({ deletedAt: null })
209209
.where(eq(knowledgeBase.id, ids.knowledgeBaseId))
210210
}
211211
})

apps/sim/lib/knowledge/__integration__/storage-cleanup.integration.ts

Lines changed: 35 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ import {
3030
createKnowledgeAclFixtureIds,
3131
seedKnowledgeAclFixture,
3232
} from '@/lib/knowledge/__integration__/seed-source-access-fixture'
33-
import { createSingleDocument, hardDeleteDocuments } from '@/lib/knowledge/documents/service'
33+
import { hardDeleteDocuments } from '@/lib/knowledge/documents/service'
3434
import {
3535
cleanupKnowledgeStorage,
3636
enqueueKnowledgeStorageCleanup,
@@ -245,59 +245,42 @@ describe('knowledge backing storage cleanup in PostgreSQL', () => {
245245
}
246246
})
247247

248-
it('cleans a legacy personal KB document using its canonical user-owned binding', async () => {
249-
const fixture = await seed()
250-
await db
251-
.update(knowledgeBase)
252-
.set({ workspaceId: null })
253-
.where(eq(knowledgeBase.id, fixture.knowledgeBaseId))
254-
await db
255-
.update(workspaceFiles)
256-
.set({ workspaceId: null })
257-
.where(eq(workspaceFiles.id, fixture.binding.id))
258-
expect(await hardDeleteDocuments([fixture.docId], 'personal-cleanup')).toBe(1)
259-
const event = await cleanupEvent(fixture.docId)
260-
expect(event.payload).toMatchObject({
261-
userId: fixture.aliceId,
262-
workspaceId: null,
263-
organizationId: null,
264-
})
265-
expect(await processOutboxEventById(event.id, handlers)).toBe('completed')
266-
expect(await getFileMetadataByKeys([fixture.key], 'knowledge-base')).toEqual([])
267-
await expect(access(fixture.filePath)).rejects.toMatchObject({ code: 'ENOENT' })
268-
await expect(
269-
createSingleDocument(
270-
{
271-
filename: 'expired.txt',
272-
fileUrl: fixture.fileUrl,
273-
fileSize: 25,
274-
mimeType: 'text/plain',
248+
it.each([false, true])(
249+
'handles already-queued personal cleanup after KB ownership repair (owner changed: %s)',
250+
async (ownerChanged) => {
251+
const fixture = await seed()
252+
await db.delete(document).where(eq(document.id, fixture.docId))
253+
await db
254+
.update(workspaceFiles)
255+
.set({ workspaceId: null, userId: ownerChanged ? fixture.bobId : fixture.aliceId })
256+
.where(eq(workspaceFiles.id, fixture.binding.id))
257+
const eventId = `knowledge-storage-cleanup:${generateId()}`
258+
events.push(eventId)
259+
await db.insert(outboxEvent).values({
260+
id: eventId,
261+
eventType: KNOWLEDGE_STORAGE_CLEANUP_EVENT,
262+
payload: {
263+
version: 1,
264+
documentId: fixture.docId,
265+
fileId: fixture.binding.id,
266+
key: fixture.key,
267+
contentUpdatedAt: fixture.binding.contentUpdatedAt.toISOString(),
268+
userId: fixture.aliceId,
269+
workspaceId: null,
270+
organizationId: null,
275271
},
276-
fixture.knowledgeBaseId,
277-
'personal-expired-upload',
278-
fixture.aliceId
279-
)
280-
).rejects.toThrow('not owned')
281-
})
272+
})
282273

283-
it('rolls back personal document deletion when the file belongs to a different user', async () => {
284-
const fixture = await seed()
285-
await db
286-
.update(knowledgeBase)
287-
.set({ workspaceId: null })
288-
.where(eq(knowledgeBase.id, fixture.knowledgeBaseId))
289-
await db
290-
.update(workspaceFiles)
291-
.set({ workspaceId: null, userId: fixture.bobId })
292-
.where(eq(workspaceFiles.id, fixture.binding.id))
293-
await expect(hardDeleteDocuments([fixture.docId], 'personal-mismatch')).rejects.toThrow(
294-
'ownership binding'
295-
)
296-
expect(
297-
await db.select({ id: document.id }).from(document).where(eq(document.id, fixture.docId))
298-
).toHaveLength(1)
299-
await expect(access(fixture.filePath)).resolves.toBeUndefined()
300-
})
274+
expect(await processOutboxEventById(eventId, handlers)).toBe('completed')
275+
if (ownerChanged) {
276+
expect(await getFileMetadataByKeys([fixture.key], 'knowledge-base')).toHaveLength(1)
277+
await expect(access(fixture.filePath)).resolves.toBeUndefined()
278+
} else {
279+
expect(await getFileMetadataByKeys([fixture.key], 'knowledge-base')).toEqual([])
280+
await expect(access(fixture.filePath)).rejects.toMatchObject({ code: 'ENOENT' })
281+
}
282+
}
283+
)
301284

302285
it('allows a create-only re-upload to register a new version after cleanup tombstones its old binding', async () => {
303286
const fixture = await seed()

apps/sim/lib/knowledge/access/scope.test.ts

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -143,12 +143,10 @@ describe('resolveKnowledgeAccessScope', () => {
143143
})
144144
})
145145

146-
it('does not query for a legacy personal knowledge base', async () => {
147-
await expect(resolveKnowledgeAccessScope(SESSION, {})).resolves.toEqual({
148-
kind: 'user',
149-
userId: 'user-1',
150-
tokens: ['pub', 'ws'],
151-
})
146+
it('rejects missing ownership before querying document access', async () => {
147+
await expect(resolveKnowledgeAccessScope(SESSION, {})).rejects.toThrow(
148+
'Resource requires exactly one workspace or organization owner'
149+
)
152150
expect(dbChainMockFns.select).not.toHaveBeenCalled()
153151
})
154152

apps/sim/lib/knowledge/access/scope.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ async function loadExternalGroupTokens(
117117
}
118118

119119
export interface KnowledgeAccessScopeContext {
120-
/** Undefined only for a legacy personal knowledge base, which cannot own connectors. */
120+
/** Exactly one workspace or organization owner is required at resolution. */
121121
workspaceId?: string
122122
organizationId?: string
123123
}
@@ -136,7 +136,6 @@ async function loadUserAccessTokens(
136136
context: KnowledgeAccessScopeContext
137137
): Promise<string[]> {
138138
const { workspaceId, organizationId } = context
139-
if (!workspaceId && !organizationId) return [...WORKSPACE_ACCESS_TOKENS]
140139
const scope = resourceScopeFromOwner(context)
141140
const baseline = organizationId ? ORGANIZATION_ACCESS_TOKENS : WORKSPACE_ACCESS_TOKENS
142141

@@ -277,6 +276,7 @@ export async function resolveKnowledgeAccessScope(
277276
'Credential Group enrollments cannot read knowledge documents'
278277
)
279278
}
279+
resourceScopeFromOwner(context)
280280
const subject = resolvePrincipalSubject(principal)
281281
if (subject?.kind !== 'sim_user') {
282282
if (context.organizationId)

apps/sim/lib/knowledge/application/authorization.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,6 @@ export interface KnowledgeAuthorizationContext
2020
organizationId?: undefined
2121
}
2222

23-
export interface LegacyPersonalKnowledgeAuthorizationContext extends KnowledgeResourceIdentifiers {
24-
workspaceId: undefined
25-
organizationId?: undefined
26-
legacyPersonalOwnerUserId: string
27-
}
28-
2923
export interface KnowledgeOrganizationAuthorizationContext extends KnowledgeResourceIdentifiers {
3024
organizationId: string
3125
workspaceId: undefined
@@ -34,7 +28,6 @@ export interface KnowledgeOrganizationAuthorizationContext extends KnowledgeReso
3428
export type KnowledgeResourceAuthorizationContext =
3529
| KnowledgeAuthorizationContext
3630
| KnowledgeOrganizationAuthorizationContext
37-
| LegacyPersonalKnowledgeAuthorizationContext
3831

3932
export type KnowledgeAuthorizationOptions = Omit<
4033
WorkspaceAuthorizationOptions<KnowledgeAuthorizationContext>,

0 commit comments

Comments
 (0)