Skip to content

Commit 11fe848

Browse files
authored
fix(knowledge): list knowledge bases on the same authority that creates them (#6770)
* fix(knowledge): list a workspace's bases on the same authority that creates them GET /api/knowledge authorizes the session against the canonical workspace and then re-derives access inside the row query from a `permissions` join. Those two no longer agree: workspace `admin` can come from an organization role alone, with no workspace permission row behind it. Such a caller passes authorization, creates a knowledge base, and then sees an empty list forever — the row is filtered out by the join. Tables and files carry no equivalent join, which is why only knowledge is affected. Read the workspace's own rows through `getWorkspaceKnowledgeBases` once the operation is authorized. The caller-scoped query stays only on the path with no workspace to authorize against. * refactor(knowledge): stop re-deriving workspace access inside the list query The module resolves workspace authority one way everywhere — an explicit permission row OR an organization admin role — except in the listing query, which joined `permissions` and required a row. Both list surfaces authorized the caller and then contradicted that authorization: an org admin could create a knowledge base through /api/knowledge or /api/v1/knowledge and never see it listed. Tables and files carry no such join. Replace the caller-scoped query with `getLegacyPersonalKnowledgeBases`, which answers only for workspace-less bases whose creator IS their only authority, and have both surfaces read the workspace's own rows through `getWorkspaceKnowledgeBases` after authorizing. The legacy rows keep riding along so they stay reachable. The permissions join now appears nowhere in the module, and the duplicated connector projection collapses onto the shared helper that enforces the row cap. * refactor(knowledge): clean up the module's client layer and fix two state bugs Cleanup pass over the knowledge module — effects, state, memo, callback, React Query, url-state, emcn, and comments — keeping the fixes that change behavior for the better and leaving the ones that would change how the UI feels. Bugs found and fixed: - Opening a document flashed "Document not ready" for a frame. The chunk-row builder rendered the loading state as a status claim: with no document loaded yet it fell through to the branch that reports a missing processing status. - A partial upload failure skipped every cache invalidation, because the throw jumped past them, so the list stayed missing rows the server had already created. Admission failures create nothing and still skip the refetch. - The document and chunk context menus captured the row they opened on, so the Enable/Disable label went stale under the list's own polling. They hold an id and resolve against live data now. - The action bar's "Select all"/"Clear" links were painted with `--brand-primary`, which is defined nowhere: the links fell back to `currentColor` and were indistinguishable from the text beside them. Consistency and weight: - Mutations no longer invalidate `detail` non-exactly for writes that touch one document: that key is the parent of every documents page, chunk page, tag definition, and connector row cached for the base. - Dead hook surface removed (five exports with no consumer, a query instantiated only to reach a cache helper, a `goToPage` that only range-checked), unused parameters dropped, `getErrorMessage` replacing hand-rolled instanceof checks. - `page` joins the document list's param group, so a search resets pagination in the same debounced write instead of writing the URL on every keystroke. - Icons import from `@sim/emcn/icons`, the action bar composes `chipFilledFillTokens` instead of restating it three times, chunk cells use the canonical content-label chrome, and the icon-only buttons have accessible names. * refactor(knowledge): one row reader, one visible-list composition Follow-up from the quality pass. The two list queries had grown into near-copies of each other — same 14-column projection, same document join, same cap check, same row mapping — and the workspace-plus-legacy composition was pasted into both the internal use case and the v1 route, one of which is a surface adapter that should not be composing domain reads at all. Both queries now read through one private projection, so a column added to one list cannot go missing from the other half of the same rendered list, and `listWorkspaceAndLegacyKnowledgeBases` owns the composition both surfaces call. That merge also projects connector types once over the merged set instead of once per source, and skips the copy-and-sort entirely when there are no legacy rows — the common case. Also from the review: the chunk-row memo depends on the two primitives it reads rather than the whole polled document object, the selected chunk resolves in one scan instead of two, an aborted chunk-search pagination throws instead of caching a truncated result as complete, upload cache reconciliation no longer delays the rejected promise, the key-hierarchy rule is stated once on the key factory rather than six times at its call sites, and `TagDefinition` has one declaration. * fix(knowledge): refresh the document list pages after a document write Review caught a regression in the invalidation narrowing: `documents` (the list pages) and `document` (one row) are SIBLINGS under `detail`, not parent and child, so scoping a write to the row key left every list rendering the filename, status, tags, `tokenCount`, and `chunkCount` it had just changed. The key factory now exposes a `documentLists` prefix and all six document-scoped mutations invalidate it alongside the row — the chunk mutations included, since every chunk write moves the parent document's `tokenCount`. `detail` stays `exact: true` where only the base's own totals move. Also repoints the shared list-convention test at `getWorkspaceKnowledgeBases`; it exercised the caller-scoped query this branch removed.
1 parent ad83796 commit 11fe848

37 files changed

Lines changed: 719 additions & 737 deletions

File tree

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

Lines changed: 4 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 { getKnowledgeBases } from '@/lib/knowledge/service'
13+
import { listWorkspaceAndLegacyKnowledgeBases } from '@/lib/knowledge/service'
1414
import { formatKnowledgeBase, handleError } from '@/app/api/v1/knowledge/utils'
1515
import {
1616
authenticateRequest,
@@ -43,7 +43,9 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
4343
const accessError = await validateWorkspaceAccess(rateLimit, userId, workspaceId)
4444
if (accessError) return accessError
4545

46-
const knowledgeBases = await getKnowledgeBases(userId, workspaceId)
46+
/** Read only after `validateWorkspaceAccess` authorized this caller; same list the
47+
* internal surface serves, from the same place. */
48+
const knowledgeBases = await listWorkspaceAndLegacyKnowledgeBases(userId, workspaceId)
4749

4850
return NextResponse.json({
4951
success: true,

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/components/document-tags-modal/document-tags-modal.tsx

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@ import {
1414
ChipModalHeader,
1515
handleKeyboardActivation,
1616
Label,
17-
Trash,
1817
} from '@sim/emcn'
18+
import { Trash } from '@sim/emcn/icons'
1919
import { createLogger } from '@sim/logger'
2020
import { formatDate } from '@sim/utils/formatting'
2121
import {
@@ -378,11 +378,7 @@ export function DocumentTagsModal({
378378

379379
return (
380380
<ChipModal open={open} onOpenChange={handleClose} srTitle='Document Tags' size='sm'>
381-
<ChipModalHeader onClose={() => handleClose(false)}>
382-
<div className='flex items-center justify-between'>
383-
<span>Document Tags</span>
384-
</div>
385-
</ChipModalHeader>
381+
<ChipModalHeader onClose={() => handleClose(false)}>Document Tags</ChipModalHeader>
386382

387383
<ChipModalBody>
388384
<ChipModalField type='custom' title='Tags'>

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx

Lines changed: 78 additions & 92 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,17 @@
11
'use client'
22

33
import { useCallback, useEffect, useEffectEvent, useMemo, useRef, useState } from 'react'
4-
import { Badge, ChipCombobox, ChipConfirmModal, Plus, Trash } from '@sim/emcn'
5-
import { ChevronDown, ChevronUp, Database, FileText, Pencil, TagIcon } from '@sim/emcn/icons'
4+
import { Badge, ChipCombobox, ChipConfirmModal, chipContentLabelClass, cn } from '@sim/emcn'
5+
import {
6+
ChevronDown,
7+
ChevronUp,
8+
Database,
9+
FileText,
10+
Pencil,
11+
Plus,
12+
TagIcon,
13+
Trash,
14+
} from '@sim/emcn/icons'
615
import { createLogger } from '@sim/logger'
716
import { truncate } from '@sim/utils/string'
817
import { useParams, useRouter } from 'next/navigation'
@@ -204,7 +213,6 @@ export function Document({
204213
chunks: initialChunks,
205214
currentPage: initialPage,
206215
totalPages: initialTotalPages,
207-
goToPage: initialGoToPage,
208216
error: initialError,
209217
updateChunk: initialUpdateChunk,
210218
} = useDocumentChunks(
@@ -292,26 +300,22 @@ export function Document({
292300
const totalPagesRef = useRef(totalPages)
293301
totalPagesRef.current = totalPages
294302

295-
const goToPage = useCallback(
296-
async (page: number) => {
297-
await setDocumentParams({ page })
298-
299-
if (showingSearch) {
300-
return
301-
}
302-
return initialGoToPage(page)
303-
},
304-
[showingSearch, initialGoToPage, setDocumentParams]
305-
)
303+
const goToPage = useCallback((page: number) => setDocumentParams({ page }), [setDocumentParams])
306304

307305
const updateChunk = showingSearch
308306
? (_id: string, _updates: Record<string, unknown>) => {}
309307
: initialUpdateChunk
310308

311309
const [chunkToDelete, setChunkToDelete] = useState<ChunkData | null>(null)
312-
const [isDeleteModalOpen, setIsDeleteModalOpen] = useState(false)
313310
const [showDeleteDocumentDialog, setShowDeleteDocumentDialog] = useState(false)
314-
const [contextMenuChunk, setContextMenuChunk] = useState<ChunkData | null>(null)
311+
const [contextMenuChunkId, setContextMenuChunkId] = useState<string | null>(null)
312+
/**
313+
* The id, not the row: the chunk list polls while a document processes, and a menu that
314+
* captured the row on open would keep offering "Enable" for a chunk already enabled.
315+
*/
316+
const contextMenuChunk = contextMenuChunkId
317+
? (displayChunks.find((chunk) => chunk.id === contextMenuChunkId) ?? null)
318+
: null
315319

316320
const { mutate: updateChunkMutation } = useUpdateChunk()
317321
const { mutate: deleteDocumentMutation, isPending: isDeletingDocument } = useDeleteDocument()
@@ -351,15 +355,10 @@ export function Document({
351355

352356
const isInEditorView = selectedChunkId !== null || isCreatingNewChunk
353357

354-
const selectedChunk = useMemo(
355-
() => (selectedChunkId ? (displayChunks.find((c) => c.id === selectedChunkId) ?? null) : null),
356-
[selectedChunkId, displayChunks]
357-
)
358-
359-
const currentChunkIndex = useMemo(
360-
() => (selectedChunk ? displayChunks.findIndex((c) => c.id === selectedChunk.id) : -1),
361-
[selectedChunk, displayChunks]
362-
)
358+
const currentChunkIndex = selectedChunkId
359+
? displayChunks.findIndex((chunk) => chunk.id === selectedChunkId)
360+
: -1
361+
const selectedChunk = currentChunkIndex >= 0 ? displayChunks[currentChunkIndex] : null
363362
const canNavigatePrev = currentChunkIndex > 0 || currentPage > 1
364363
const canNavigateNext = currentChunkIndex < displayChunks.length - 1 || currentPage < totalPages
365364

@@ -402,14 +401,14 @@ export function Document({
402401
}
403402
}, [isDirty, isCreatingNewChunk])
404403

405-
const handleUnsavedChangesOpenChange = useCallback((open: boolean) => {
404+
const handleUnsavedChangesOpenChange = (open: boolean) => {
406405
if (!open) {
407406
setShowUnsavedChangesAlert(false)
408407
setPendingAction(null)
409408
}
410-
}, [])
409+
}
411410

412-
const handleDiscardChanges = useCallback(() => {
411+
const handleDiscardChanges = () => {
413412
setShowUnsavedChangesAlert(false)
414413
const action = pendingAction
415414
setPendingAction(null)
@@ -419,7 +418,7 @@ export function Document({
419418
} else {
420419
closeEditor()
421420
}
422-
}, [pendingAction, closeEditor])
421+
}
423422

424423
const handleSaveEvent = useEffectEvent(handleSave)
425424

@@ -646,7 +645,6 @@ export function Document({
646645
if (found) {
647646
setSelectedChunkId(chunkId)
648647
} else if (!navigatedToNewPage && totalPagesRef.current > totalPages) {
649-
// A new page was created — navigate to it
650648
navigatedToNewPage = true
651649
retries = 0
652650
void goToPage(totalPagesRef.current)
@@ -681,10 +679,8 @@ export function Document({
681679
}
682680
: undefined
683681

684-
const enabledDisplayLabel = useMemo(() => {
685-
if (enabledFilter.length === 0) return 'All'
686-
return enabledFilter[0] === 'enabled' ? 'Enabled' : 'Disabled'
687-
}, [enabledFilter])
682+
const enabledDisplayLabel =
683+
enabledFilter.length === 0 ? 'All' : enabledFilter[0] === 'enabled' ? 'Enabled' : 'Disabled'
688684

689685
const filterContent = useMemo(
690686
() => (
@@ -724,7 +720,7 @@ export function Document({
724720
)}
725721
</div>
726722
),
727-
[enabledFilter, enabledDisplayLabel, setEnabledFilter]
723+
[enabledFilter, setEnabledFilter]
728724
)
729725

730726
const filterTags: FilterTag[] = useMemo(
@@ -746,31 +742,22 @@ export function Document({
746742
[setSelectedChunkId]
747743
)
748744

749-
const handleToggleEnabled = useCallback(
750-
(chunkId: string) => {
751-
const chunk = displayChunks.find((c) => c.id === chunkId)
752-
if (!chunk) return
745+
const handleToggleEnabled = (chunkId: string) => {
746+
const chunk = displayChunks.find((c) => c.id === chunkId)
747+
if (!chunk) return
753748

754-
const newEnabled = !chunk.enabled
755-
updateChunk(chunkId, { enabled: newEnabled })
756-
updateChunkMutation(
757-
{ knowledgeBaseId, documentId, chunkId, enabled: newEnabled },
758-
{ onError: () => updateChunk(chunkId, { enabled: chunk.enabled }) }
759-
)
760-
},
761-
[displayChunks, knowledgeBaseId, documentId, updateChunk]
762-
)
749+
const newEnabled = !chunk.enabled
750+
updateChunk(chunkId, { enabled: newEnabled })
751+
updateChunkMutation(
752+
{ knowledgeBaseId, documentId, chunkId, enabled: newEnabled },
753+
{ onError: () => updateChunk(chunkId, { enabled: chunk.enabled }) }
754+
)
755+
}
763756

764-
const handleDeleteChunk = useCallback(
765-
(chunkId: string) => {
766-
const chunk = displayChunks.find((c) => c.id === chunkId)
767-
if (chunk) {
768-
setChunkToDelete(chunk)
769-
setIsDeleteModalOpen(true)
770-
}
771-
},
772-
[displayChunks]
773-
)
757+
const handleDeleteChunk = (chunkId: string) => {
758+
const chunk = displayChunks.find((c) => c.id === chunkId)
759+
if (chunk) setChunkToDelete(chunk)
760+
}
774761

775762
const handleCloseDeleteModal = () => {
776763
if (chunkToDelete) {
@@ -780,7 +767,6 @@ export function Document({
780767
return newSet
781768
})
782769
}
783-
setIsDeleteModalOpen(false)
784770
setChunkToDelete(null)
785771
}
786772

@@ -863,17 +849,14 @@ export function Document({
863849
performBulkChunkOperation('delete', chunksToDelete)
864850
}
865851

866-
const [enabledCount, disabledCount] = useMemo(() => {
867-
let enabled = 0
868-
let disabled = 0
869-
for (const chunk of displayChunks) {
870-
if (selectedChunks.has(chunk.id)) {
871-
if (chunk.enabled) enabled++
872-
else disabled++
873-
}
852+
let enabledCount = 0
853+
let disabledCount = 0
854+
for (const chunk of displayChunks) {
855+
if (selectedChunks.has(chunk.id)) {
856+
if (chunk.enabled) enabledCount++
857+
else disabledCount++
874858
}
875-
return [enabled, disabled]
876-
}, [displayChunks, selectedChunks])
859+
}
877860

878861
const isAllSelected = displayChunks.length > 0 && selectedChunks.size === displayChunks.length
879862

@@ -890,7 +873,7 @@ export function Document({
890873
}
891874
}
892875

893-
setContextMenuChunk(chunk)
876+
setContextMenuChunkId(chunk.id)
894877
baseHandleContextMenu(e)
895878
},
896879
[
@@ -902,18 +885,15 @@ export function Document({
902885
]
903886
)
904887

905-
const handleEmptyContextMenu = useCallback(
906-
(e: React.MouseEvent) => {
907-
setContextMenuChunk(null)
908-
baseHandleContextMenu(e)
909-
},
910-
[baseHandleContextMenu]
911-
)
888+
const handleEmptyContextMenu = (e: React.MouseEvent) => {
889+
setContextMenuChunkId(null)
890+
baseHandleContextMenu(e)
891+
}
912892

913-
const handleContextMenuClose = useCallback(() => {
893+
const handleContextMenuClose = () => {
914894
closeContextMenu()
915-
setContextMenuChunk(null)
916-
}, [closeContextMenu])
895+
setContextMenuChunkId(null)
896+
}
917897

918898
const selectableConfig: SelectableConfig | undefined = isCompleted
919899
? {
@@ -955,7 +935,17 @@ export function Document({
955935
[activeSort, onSortColumn, onClearSort, goToPage]
956936
)
957937

938+
const hasDocumentData = documentData !== null
939+
const processingStatus = documentData?.processingStatus
940+
958941
const chunkRows: ResourceRow[] = useMemo(() => {
942+
/**
943+
* No document yet is "not known", not "not ready". Falling through to the status row
944+
* flashed `Document not ready` on every open, for the frame between mount and the
945+
* document query resolving — a claim about a document nothing had read yet.
946+
*/
947+
if (!hasDocumentData) return []
948+
959949
if (!isCompleted) {
960950
return [
961951
{
@@ -966,12 +956,10 @@ export function Document({
966956
<div className='flex items-center gap-2'>
967957
<FileText className='size-5 flex-shrink-0 text-[var(--text-muted)]' />
968958
<span className='text-[var(--text-muted)] text-sm italic'>
969-
{documentData?.processingStatus === 'pending' &&
970-
'Document processing pending...'}
971-
{documentData?.processingStatus === 'processing' &&
972-
'Document processing in progress...'}
973-
{documentData?.processingStatus === 'failed' && 'Document processing failed'}
974-
{!documentData?.processingStatus && 'Document not ready'}
959+
{processingStatus === 'pending' && 'Document processing pending...'}
960+
{processingStatus === 'processing' && 'Document processing in progress...'}
961+
{processingStatus === 'failed' && 'Document processing failed'}
962+
{!processingStatus && 'Document not ready'}
975963
</span>
976964
</div>
977965
),
@@ -992,16 +980,14 @@ export function Document({
992980
cells: {
993981
content: {
994982
content: (
995-
<span className='block truncate text-[var(--text-primary)] text-sm'>
983+
<span className={cn('block', chipContentLabelClass)}>
996984
<SearchHighlight text={previewContent} searchQuery={searchQuery} />
997985
</span>
998986
),
999987
},
1000988
index: {
1001989
content: (
1002-
<span className='font-mono text-[var(--text-primary)] text-sm'>
1003-
{chunk.chunkIndex}
1004-
</span>
990+
<span className={cn('font-mono', chipContentLabelClass)}>{chunk.chunkIndex}</span>
1005991
),
1006992
},
1007993
tokens: {
@@ -1017,7 +1003,7 @@ export function Document({
10171003
},
10181004
}
10191005
})
1020-
}, [isCompleted, documentData?.processingStatus, displayChunks, searchQuery])
1006+
}, [isCompleted, hasDocumentData, processingStatus, displayChunks, searchQuery])
10211007

10221008
const saveLabel =
10231009
saveStatus === 'saving'
@@ -1232,7 +1218,7 @@ export function Document({
12321218
chunk={chunkToDelete}
12331219
knowledgeBaseId={knowledgeBaseId}
12341220
documentId={documentId}
1235-
isOpen={isDeleteModalOpen}
1221+
isOpen={chunkToDelete !== null}
12361222
onClose={handleCloseDeleteModal}
12371223
/>
12381224

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/loading.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use client'
22

3-
import { Plus } from '@sim/emcn'
4-
import { Database, FileText } from '@sim/emcn/icons'
3+
import { Database, FileText, Plus } from '@sim/emcn/icons'
54
import { noop } from '@sim/utils/helpers'
65
import {
76
type BreadcrumbItem,

0 commit comments

Comments
 (0)