From bc7d11416dfa5a3bbc88d2e0cdebbae00ed60e1f Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sun, 16 Aug 2026 22:47:18 -0700 Subject: [PATCH] fix(knowledge): stop capping the unpaged knowledge-base list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #6770 routed GET /api/knowledge through the workspace read, which carried a 10,000-row cap. Staging crossed it, and the first list request after the deploy returned 500 with "Knowledge base list exceeds the 10000 row limit" — against data that had served fine for days. The cap could never have worked. Its throw was guarded by `limit === undefined`, so it fired only for callers that had NOT asked for a page: exactly the callers with no cursor to retry with and no way to ask for less. A paged caller never reached it. It also read one row PAST the cap before throwing, so it refused to serve rows it had already materialized — most of the memory was already spent. Soft-delete cleanup reclaims archived rows only past a retention window, and not at all where none is configured, so a workspace that archives faster than that window crosses any fixed count on its own. Remove it. An unpaged read is unbounded, matching the sibling internal lists (`listTables`, workspace files), and paged callers keep their page. That fixes the same latent 500 in the archived list, the catalog read, and the VFS name lookup, which are all unpaged too, rather than only the surface that failed. Two more of the same shape found while auditing for others: - `attachConnectorTypes` threw a bare Error above its own cap, on those same unpaged callers. Archiving a knowledge base archives its connectors, so the growth curve that broke staging could not reach it — but it is the identical construct, and the sibling `latestJobsForTables` has no equivalent. - The VFS path lookup read every knowledge base whose name merely CONTAINED the term and then exact-matched in JS, so a single-row lookup scaled with the workspace. It now queries the exact name and reads two rows, mirroring `findActiveTablesByExactName`. --- .../knowledge/application/knowledge-vfs.ts | 9 +- apps/sim/lib/knowledge/constants.ts | 10 -- apps/sim/lib/knowledge/service.test.ts | 112 ++++++++++++++---- apps/sim/lib/knowledge/service.ts | 63 ++++------ 4 files changed, 117 insertions(+), 77 deletions(-) diff --git a/apps/sim/lib/knowledge/application/knowledge-vfs.ts b/apps/sim/lib/knowledge/application/knowledge-vfs.ts index f7fbff3e8d7..1f5444394d3 100644 --- a/apps/sim/lib/knowledge/application/knowledge-vfs.ts +++ b/apps/sim/lib/knowledge/application/knowledge-vfs.ts @@ -9,7 +9,7 @@ import { import { knowledgeOperations } from '@/lib/knowledge/application/operations' import { deleteKnowledgeBase, - getWorkspaceKnowledgeBases, + findActiveKnowledgeBasesByExactName, updateKnowledgeBase, } from '@/lib/knowledge/service' import type { KnowledgeBaseWithCounts } from '@/lib/knowledge/types' @@ -28,11 +28,8 @@ export type DeleteKnowledgeBaseByVfsPathInput = KnowledgeVfsReferenceInput async function resolveKnowledgeBaseByVfsName( context: KnowledgeWorkspaceContext, sourceName: string -): Promise { - const { data: rows } = await getWorkspaceKnowledgeBases(context.workspaceId, 'active', { - search: sourceName, - }) - const matches = rows.filter((row) => row.name === sourceName) +): Promise> { + const matches = await findActiveKnowledgeBasesByExactName(context.workspaceId, sourceName) if (matches.length > 1) { throw new OrchestrationError( 'conflict', diff --git a/apps/sim/lib/knowledge/constants.ts b/apps/sim/lib/knowledge/constants.ts index 3e6310eba32..231833abde5 100644 --- a/apps/sim/lib/knowledge/constants.ts +++ b/apps/sim/lib/knowledge/constants.ts @@ -2,15 +2,7 @@ import { MAX_FOLDERS_PER_WORKSPACE } from '@/lib/folders/constants' /** Max character length for a knowledge base description, enforced at every layer (UI, internal API, v1 API). */ export const KNOWLEDGE_BASE_DESCRIPTION_MAX_LENGTH = 10_000 -/** Hard bound for full-workspace knowledge-base list projections. */ -export const MAX_KNOWLEDGE_BASES_PER_WORKSPACE = 10_000 -/** - * Cap on one caller's legacy workspace-less knowledge bases. Separate from the per-workspace - * cap because it bounds a per-user set governed by no workspace rule — the two limits should - * be free to move independently. - */ -export const MAX_LEGACY_PERSONAL_KNOWLEDGE_BASES = 10_000 /** Hard bound for path-indexed knowledge folder trees and recursive cascades. */ export const MAX_KNOWLEDGE_FOLDERS_PER_WORKSPACE = MAX_FOLDERS_PER_WORKSPACE @@ -20,8 +12,6 @@ export const MAX_KNOWLEDGE_FOLDERS_PER_WORKSPACE = MAX_FOLDERS_PER_WORKSPACE * bound their id arrays without pulling a server-only module into client code. */ export const MAX_KNOWLEDGE_BATCH_ITEMS = 100 -/** Hard bound for connector-type rows projected onto one knowledge-base list. */ -export const MAX_KNOWLEDGE_CONNECTOR_TYPE_ROWS_PER_LIST = 100_000 /** Maximum documents accepted by one internal bulk-create command. */ export const MAX_KNOWLEDGE_DOCUMENTS_PER_CREATE = 100 /** Maximum connector documents mutated atomically by one command. */ diff --git a/apps/sim/lib/knowledge/service.test.ts b/apps/sim/lib/knowledge/service.test.ts index 584633d9c1a..cdea996fef8 100644 --- a/apps/sim/lib/knowledge/service.test.ts +++ b/apps/sim/lib/knowledge/service.test.ts @@ -39,8 +39,8 @@ vi.mock('@/lib/billing/core/usage', () => ({ ensureUserStatsExists: mockEnsureUserStatsExists, })) -import { MAX_KNOWLEDGE_BASES_PER_WORKSPACE } from '@/lib/knowledge/constants' import { + findActiveKnowledgeBasesByExactName, getLegacyPersonalKnowledgeBases, getWorkspaceKnowledgeBases, KnowledgeBasePermissionError, @@ -48,23 +48,49 @@ import { updateKnowledgeBase, } from '@/lib/knowledge/service' -describe('getWorkspaceKnowledgeBases — bounded reads', () => { +/** + * A row cap on this read could only ever fire for a caller that did NOT ask for a page — the + * one kind of caller with no cursor to act on it — so an oversized workspace has to be served, + * not refused. + */ +describe('getWorkspaceKnowledgeBases — paging', () => { beforeEach(() => { vi.clearAllMocks() resetDbChainMock() }) - it('fails before projecting connector data for an oversized workspace list', async () => { - dbChainMockFns.limit.mockResolvedValueOnce( - Array.from({ length: MAX_KNOWLEDGE_BASES_PER_WORKSPACE + 1 }, (_, index) => ({ + it('reads unbounded when the caller asked for no page', async () => { + dbChainMockFns.orderBy.mockResolvedValueOnce( + Array.from({ length: 10_001 }, (_, index) => ({ id: `kb-${index}`, + chunkingConfig: {}, + docCount: 0, })) ) - await expect(getWorkspaceKnowledgeBases('ws-1')).rejects.toThrow( - `Knowledge base list exceeds the ${MAX_KNOWLEDGE_BASES_PER_WORKSPACE} row limit` - ) - expect(dbChainMockFns.limit).toHaveBeenCalledWith(MAX_KNOWLEDGE_BASES_PER_WORKSPACE + 1) + const result = await getWorkspaceKnowledgeBases('ws-1') + + expect(result.data).toHaveLength(10_001) + expect(dbChainMockFns.limit).not.toHaveBeenCalled() + }) + + it('reads one row past the page so it can report another page', async () => { + dbChainMockFns.limit + .mockResolvedValueOnce( + Array.from({ length: 3 }, (_, index) => ({ + id: `kb-${index}`, + chunkingConfig: {}, + docCount: 0, + createdAt: new Date('2026-01-01T00:00:00Z'), + })) + ) + .mockResolvedValueOnce([]) + + const result = await getWorkspaceKnowledgeBases('ws-1', 'active', { limit: 2 }) + + expect(dbChainMockFns.limit).toHaveBeenCalledWith(3) + expect(result.data).toHaveLength(2) + expect(result.nextCursorKeys).not.toBeNull() }) }) @@ -110,17 +136,31 @@ describe('getLegacyPersonalKnowledgeBases', () => { expect(joinedTables).toContain(schemaMock.document) expect(joinedTables).not.toContain(schemaMock.permissions) }) +}) - it('fails before projecting connector data for an oversized set', async () => { - dbChainMockFns.limit.mockResolvedValueOnce( - Array.from({ length: MAX_KNOWLEDGE_BASES_PER_WORKSPACE + 1 }, (_, index) => ({ - id: `kb-${index}`, - })) - ) +/** + * A VFS path names one knowledge base exactly. Resolving it by reading every base whose name + * merely CONTAINS the term, then filtering in JS, makes a single-row lookup scale with the + * workspace — the sibling `findActiveTablesByExactName` is the shape to match. + */ +describe('findActiveKnowledgeBasesByExactName', () => { + beforeEach(() => { + vi.clearAllMocks() + resetDbChainMock() + }) - await expect(getLegacyPersonalKnowledgeBases('user-a')).rejects.toThrow( - `Legacy personal knowledge base list exceeds the ${MAX_KNOWLEDGE_BASES_PER_WORKSPACE} row limit` - ) + it('matches the name exactly and reads at most two rows', async () => { + await findActiveKnowledgeBasesByExactName('ws-1', 'Docs') + + const [condition] = dbChainMockFns.where.mock.calls[0] ?? [] + expect( + hasMockCondition( + condition, + (node) => + node.type === 'eq' && node.left === schemaMock.knowledgeBase.name && node.right === 'Docs' + ) + ).toBe(true) + expect(dbChainMockFns.limit).toHaveBeenCalledWith(2) }) }) @@ -135,6 +175,29 @@ describe('listWorkspaceAndLegacyKnowledgeBases', () => { resetDbChainMock() }) + /** + * Soft-delete cleanup only reclaims archived rows past a retention window — and none at all + * for a workspace with no retention configured — so a workspace that archives faster than + * that window crosses any fixed count. This surface has no cursor to page with, so a cap + * here could only mean a 500 on the knowledge page and Recently Deleted, which is exactly + * what it meant on staging. + */ + it('serves a workspace whose archived set is larger than the old row cap', async () => { + const rows = Array.from({ length: 10_001 }, (_, index) => ({ + id: `kb-${index}`, + chunkingConfig: {}, + docCount: 0, + createdAt: new Date('2026-01-01T00:00:00Z'), + })) + dbChainMockFns.orderBy.mockResolvedValueOnce(rows).mockResolvedValueOnce([]) + + const result = await listWorkspaceAndLegacyKnowledgeBases('user-a', 'ws-1', 'archived') + + expect(result).toHaveLength(10_001) + /** Neither the workspace read nor the legacy read may bound itself. */ + expect(dbChainMockFns.limit).not.toHaveBeenCalled() + }) + it('orders both sources as one list and projects connectors once', async () => { const workspaceRow = { id: 'kb-workspace', @@ -148,16 +211,17 @@ describe('listWorkspaceAndLegacyKnowledgeBases', () => { docCount: 0, createdAt: new Date('2025-01-01T00:00:00Z'), } - dbChainMockFns.limit - .mockResolvedValueOnce([workspaceRow]) - .mockResolvedValueOnce([legacyRow]) - .mockResolvedValueOnce([]) + /** Both row reads are unbounded now, so each resolves at `orderBy` rather than `limit`. */ + dbChainMockFns.orderBy.mockResolvedValueOnce([workspaceRow]).mockResolvedValueOnce([legacyRow]) const result = await listWorkspaceAndLegacyKnowledgeBases('user-a', 'ws-1') expect(result.map((kb) => kb.id)).toEqual(['kb-legacy', 'kb-workspace']) - /** Two row reads and ONE connector projection — three chains, never four. */ - expect(dbChainMockFns.limit).toHaveBeenCalledTimes(3) + /** ONE connector projection over the merged set, not one per source. */ + const connectorReads = dbChainMockFns.from.mock.calls.filter( + ([table]) => table === schemaMock.knowledgeConnector + ) + expect(connectorReads).toHaveLength(1) }) }) diff --git a/apps/sim/lib/knowledge/service.ts b/apps/sim/lib/knowledge/service.ts index 2af47ea2525..16c710d4817 100644 --- a/apps/sim/lib/knowledge/service.ts +++ b/apps/sim/lib/knowledge/service.ts @@ -28,11 +28,6 @@ import { import { OrchestrationError } from '@/lib/core/orchestration/types' import { generateRestoreName } from '@/lib/core/utils/restore-name' import { findActiveFolder, resolveRestoredFolderId } from '@/lib/folders/queries' -import { - MAX_KNOWLEDGE_BASES_PER_WORKSPACE, - MAX_KNOWLEDGE_CONNECTOR_TYPE_ROWS_PER_LIST, - MAX_LEGACY_PERSONAL_KNOWLEDGE_BASES, -} from '@/lib/knowledge/constants' import type { ChunkingConfig, CreateKnowledgeBaseData, @@ -155,11 +150,7 @@ export interface GetKnowledgeBasesOptions { search?: string sortBy?: V2KnowledgeBaseSortBy sortOrder?: ListSortOrder - /** - * Page size. Omitted reads the whole workspace set as one page, capped by - * {@link MAX_KNOWLEDGE_BASES_PER_WORKSPACE} — what the internal callers that - * need every row still do. - */ + /** Page size. Omitted reads the whole set as one page. */ limit?: number /** Keyset to resume after, from the previous page's `nextCursorKeys`. */ cursorKeys?: CursorKey[] @@ -181,9 +172,9 @@ function knowledgeBaseScopeCondition(scope: KnowledgeBaseScope) { async function readKnowledgeBaseRows( where: SQL | undefined, orderBy: SQL[], - limit: number + limit?: number ): Promise>> { - const rows = await db + const query = db .select({ id: knowledgeBase.id, userId: knowledgeBase.userId, @@ -213,7 +204,8 @@ async function readKnowledgeBaseRows( .where(where) .groupBy(knowledgeBase.id) .orderBy(...orderBy) - .limit(limit) + + const rows = limit === undefined ? await query : await query.limit(limit) return rows.map((kb) => ({ ...kb, @@ -241,13 +233,7 @@ async function attachConnectorTypes( isNull(knowledgeConnector.deletedAt) ) ) - .limit(MAX_KNOWLEDGE_CONNECTOR_TYPE_ROWS_PER_LIST + 1) : [] - if (connectorRows.length > MAX_KNOWLEDGE_CONNECTOR_TYPE_ROWS_PER_LIST) { - throw new Error( - `Knowledge connector projection exceeds the ${MAX_KNOWLEDGE_CONNECTOR_TYPE_ROWS_PER_LIST} row limit` - ) - } const connectorTypesByKb = new Map() for (const row of connectorRows) { @@ -286,10 +272,11 @@ async function readWorkspaceKnowledgeBaseRows( const keys = KNOWLEDGE_BASE_SORTS[sortBy] /** - * An unpaged read still reads one row past the cap so an oversized workspace - * is a hard failure rather than a silently truncated list. + * An unpaged read is unbounded, matching the sibling internal lists (`listTables`, workspace + * files). A row cap could only ever fire for a caller that did not ask for a page — the one + * kind with no cursor to respond with — so it can only turn a slow list into a 500. */ - const readLimit = (limit ?? MAX_KNOWLEDGE_BASES_PER_WORKSPACE) + 1 + const readLimit = limit === undefined ? undefined : limit + 1 const rows = await readKnowledgeBaseRows( and( @@ -307,12 +294,6 @@ async function readWorkspaceKnowledgeBaseRows( readLimit ) - if (limit === undefined && rows.length > MAX_KNOWLEDGE_BASES_PER_WORKSPACE) { - throw new Error( - `Knowledge base list exceeds the ${MAX_KNOWLEDGE_BASES_PER_WORKSPACE} row limit` - ) - } - return keysetPage(keys, rows, limit) } @@ -348,17 +329,9 @@ async function readLegacyPersonalKnowledgeBaseRows( eq(knowledgeBase.userId, userId), isNull(knowledgeBase.workspaceId) ), - listOrderBy(keysetColumns(KNOWLEDGE_BASE_SORTS.createdAt), 'asc'), - MAX_LEGACY_PERSONAL_KNOWLEDGE_BASES + 1 + listOrderBy(keysetColumns(KNOWLEDGE_BASE_SORTS.createdAt), 'asc') ) - /** One row past the cap, so an oversized set fails loudly instead of truncating in silence. */ - if (rows.length > MAX_LEGACY_PERSONAL_KNOWLEDGE_BASES) { - throw new Error( - `Legacy personal knowledge base list exceeds the ${MAX_LEGACY_PERSONAL_KNOWLEDGE_BASES} row limit` - ) - } - return rows } @@ -401,6 +374,22 @@ export async function listWorkspaceAndLegacyKnowledgeBases( ) } +/** Loads at most two active exact-name matches so a caller can fail on corrupt ambiguity. */ +export async function findActiveKnowledgeBasesByExactName( + workspaceId: string, + name: string +): Promise>> { + return readKnowledgeBaseRows( + and( + eq(knowledgeBase.workspaceId, workspaceId), + eq(knowledgeBase.name, name), + isNull(knowledgeBase.deletedAt) + ), + listOrderBy(keysetColumns(KNOWLEDGE_BASE_SORTS.createdAt), 'asc'), + 2 + ) +} + /** * Create a new knowledge base */