Skip to content

Commit b653ddc

Browse files
committed
fix(folders): close the findings from the full-diff audit
Migration 0274 reconciles instead of catching up - The deployed manager writes `workspace_file_folders` EXCLUSIVELY, so `folder`'s file rows are frozen at the 0272 snapshot: every rename, move, delete, and restore since then lives only in the legacy table. An insert-only catch-up left all of that diverged — renames reverted, deleted folders came back as active phantoms, and a stale-active row could hold the name a newer folder legitimately took - That last case was silent: the bare `ON CONFLICT DO NOTHING` swallows a NAME violation as readily as an id one, so the newer folder was discarded and every file inside it stranded. The conflict target is now `(id)`, so a name collision — which would mean the source violated its own unique index — fails loudly instead - Reconciles by parking mirrored names first, then upserting, so no transient state can collide with the partial unique index. Both statements are idempotent, and the header says what nothing else would: this has to be run again after the deploy drains, because old pods keep writing the legacy table until they are gone `/api/folders` no longer serves file folders - Adding `'file'` to `servedFolderResourceTypeSchema` opened a second writer over the same rows. It bypassed the `workspace_file_folders:${workspaceId}` advisory lock that makes the manager's check-then-write pairs atomic, and it accepted names containing `/`, `\`, `.` and `..`, which the file surface forbids because a folder name becomes a path segment — a folder named `a/b` is then unreachable by path forever, and breadcrumbs resolve it as two segments. The storage cutover never needed a second API, so there isn't one Conflicts that returned 500 - `v1/admin/workspaces/[id]/import` created its folder with an unserialized SELECT-then- INSERT, so two imports sharing a folder path failed the whole import. The name is server-chosen, so it now adopts whichever folder won, like `ensureWorkspaceFileFolderPath` - Folder duplicate never caught 23505. `newId` is client-supplied, so replaying a duplicate whose response was lost hit the primary key and returned 500 instead of a 409 the caller can act on Performance - `knowledgeKeys` moved to `hooks/queries/utils/knowledge-keys.ts`. Reading it from `hooks/queries/kb/knowledge` pulled a ~1000-line hook module — and the `@sim/emcn` barrel behind it — into `hooks/queries/folders`, which three server prefetches import. That is the same import edge the navigation-speed audit measured at 11.8MB per workspace route; `tableKeys` already lived in a keys-only module for exactly this reason Merge drop - Knowledge never got the chrome Tables did: its prefetch now fetches the folder tree alongside the bases, so the list does not paint ungrouped and a `?folderId=` deep link does not render an empty breadcrumb, and its loading fallback declares the "New folder" action so the header does not shift on hydration Comments corrected to match reality - The shared folder row and context menu claimed Files as a consumer; Files still builds its own row and routes folders through `FileRowContextMenu`, which the TSDoc now says - `schema.ts` prescribed a row COUNT comparison before dropping the legacy table. The divergence 0274 repairs is in row CONTENTS, which a count check passes straight through
1 parent f96d853 commit b653ddc

25 files changed

Lines changed: 18489 additions & 119 deletions

File tree

apps/sim/app/api/folders/[id]/duplicate/route.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { db } from '@sim/db'
33
import { folder as folderTable, workflow } from '@sim/db/schema'
44
import { createLogger } from '@sim/logger'
55
import { FolderLockedError } from '@sim/platform-authz/workflow'
6+
import { getPostgresErrorCode } from '@sim/utils/errors'
67
import { generateId } from '@sim/utils/id'
78
import { and, eq, isNull } from 'drizzle-orm'
89
import { type NextRequest, NextResponse } from 'next/server'
@@ -200,6 +201,20 @@ export const POST = withRouteHandler(
200201
return NextResponse.json({ error: error.message }, { status: error.status })
201202
}
202203

204+
/**
205+
* `newId` is client-supplied, so replaying a duplicate whose response was lost hits the
206+
* primary key rather than the name index. A name collision is equally reachable: dedup
207+
* runs before the INSERT, but a concurrent create can take the name in between. Either
208+
* way this is a conflict the caller can act on, not a server fault.
209+
*/
210+
if (getPostgresErrorCode(error) === '23505') {
211+
logger.warn(`[${requestId}] Folder duplication conflicted`, { sourceFolderId })
212+
return NextResponse.json(
213+
{ error: 'A folder with this name already exists in this location' },
214+
{ status: 409 }
215+
)
216+
}
217+
203218
if (error instanceof FolderDuplicationError) {
204219
logger.warn(`[${requestId}] Folder duplication rejected: ${error.message}`, {
205220
sourceFolderId,

apps/sim/app/api/v1/admin/workspaces/[id]/import/route.ts

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
import { db } from '@sim/db'
2727
import { folder as folderTable, workflow } from '@sim/db/schema'
2828
import { createLogger } from '@sim/logger'
29-
import { getErrorMessage } from '@sim/utils/errors'
29+
import { getErrorMessage, getPostgresErrorCode } from '@sim/utils/errors'
3030
import { generateId } from '@sim/utils/id'
3131
import { and, eq, isNull } from 'drizzle-orm'
3232
import { NextResponse } from 'next/server'
@@ -108,16 +108,42 @@ async function ensureImportFolder(
108108
if (existing) return existing.id
109109

110110
const folderId = generateId()
111-
await db.insert(folderTable).values({
112-
id: folderId,
113-
resourceType: 'workflow',
114-
name,
115-
userId,
116-
workspaceId,
117-
parentId,
118-
createdAt: new Date(),
119-
updatedAt: new Date(),
120-
})
111+
try {
112+
await db.insert(folderTable).values({
113+
id: folderId,
114+
resourceType: 'workflow',
115+
name,
116+
userId,
117+
workspaceId,
118+
parentId,
119+
createdAt: new Date(),
120+
updatedAt: new Date(),
121+
})
122+
} catch (error) {
123+
/**
124+
* The SELECT above is not serialized against this INSERT, so two imports sharing a folder
125+
* path race here. The name is server-chosen, not user-chosen, so the right resolution is
126+
* to adopt whichever folder won rather than fail the whole import with a 500 — the same
127+
* reuse-on-conflict `ensureWorkspaceFileFolderPath` already does.
128+
*/
129+
if (getPostgresErrorCode(error) !== '23505') throw error
130+
131+
const [concurrent] = await db
132+
.select({ id: folderTable.id })
133+
.from(folderTable)
134+
.where(
135+
and(
136+
eq(folderTable.workspaceId, workspaceId),
137+
eq(folderTable.resourceType, 'workflow'),
138+
eq(folderTable.name, name),
139+
parentId ? eq(folderTable.parentId, parentId) : isNull(folderTable.parentId),
140+
isNull(folderTable.deletedAt)
141+
)
142+
)
143+
.limit(1)
144+
if (!concurrent) throw error
145+
return concurrent.id
146+
}
121147
return folderId
122148
}
123149

apps/sim/app/workspace/[workspaceId]/components/folders/folder-context-menu.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,12 @@ interface FolderContextMenuProps {
3232
}
3333

3434
/**
35-
* Row context menu for a folder, shared across every foldered resource list so a folder
36-
* offers the same actions on Knowledge, Tables, and Files.
35+
* Row context menu for a folder, shared by the resource lists built on the generic folder
36+
* engine — Knowledge and Tables — so a folder offers the same actions on both.
37+
*
38+
* Files is deliberately not a consumer: its rows carry multi-select and bulk actions, so a
39+
* folder there routes through `FileRowContextMenu` alongside the file rows it is selected
40+
* with. Converging the two is follow-up work.
3741
*
3842
* Mirrors the resource-row menus (`KnowledgeBaseContextMenu`, `FileRowContextMenu`): a
3943
* `DropdownMenu` anchored to a one-pixel fixed trigger at the cursor, non-modal so the list

apps/sim/app/workspace/[workspaceId]/components/folders/folder-row.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,10 @@ export interface FolderRowOptions {
2727
}
2828

2929
/**
30-
* Builds the canonical folder `ResourceRow` shared by every foldered list, so a folder looks
31-
* and behaves identically on Knowledge, Tables, and Files. The row id is namespaced by
30+
* Builds the canonical folder `ResourceRow` for the lists built on the generic folder engine
31+
* (Knowledge and Tables), so a folder looks and behaves identically on both. Files still
32+
* builds its own row — it carries a size roll-up and a distinct row-id scheme that also
33+
* namespaces file ids. The row id here is namespaced by
3234
* {@link folderRowId}, so the caller's click/context-menu handlers distinguish a folder from
3335
* a resource with `parseFolderedRowId` rather than a second lookup.
3436
*

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ import type {
2222
MothershipResourceType,
2323
} from '@/app/workspace/[workspaceId]/home/types'
2424
import { getBareIconStyle, type StyleableIcon } from '@/blocks/icon-color'
25-
import { knowledgeKeys } from '@/hooks/queries/kb/knowledge'
2625
import { logKeys } from '@/hooks/queries/logs'
2726
import { mothershipChatKeys } from '@/hooks/queries/mothership-chats'
2827
import { scheduleKeys } from '@/hooks/queries/schedules'
2928
import { folderKeys } from '@/hooks/queries/utils/folder-keys'
3029
import { invalidateWorkflowLists } from '@/hooks/queries/utils/invalidate-workflow-lists'
30+
import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
3131
import { tableKeys } from '@/hooks/queries/utils/table-keys'
3232
import { workspaceFileFolderKeys } from '@/hooks/queries/workspace-file-folders'
3333
import { workspaceFilesKeys } from '@/hooks/queries/workspace-files'

apps/sim/app/workspace/[workspaceId]/knowledge/hooks/use-knowledge-upload.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
WHOLE_FILE_PARALLEL_UPLOADS,
2020
} from '@/lib/uploads/client/direct-upload'
2121
import { getFileContentType, isAbortError, isNetworkError } from '@/lib/uploads/utils/file-utils'
22-
import { knowledgeKeys } from '@/hooks/queries/kb/knowledge'
22+
import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
2323

2424
const logger = createLogger('KnowledgeUpload')
2525

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

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

33
import { Plus } from '@sim/emcn'
4-
import { Database } from '@sim/emcn/icons'
4+
import { Database, FolderPlus } from '@sim/emcn/icons'
55
import {
66
type ChromeActionSpec,
77
ResourceChromeFallback,
@@ -17,7 +17,10 @@ const COLUMNS = [
1717
{ id: 'updated', header: 'Last Updated' },
1818
]
1919

20-
const ACTIONS: ChromeActionSpec[] = [{ text: 'New base', icon: Plus, variant: 'primary' }]
20+
const ACTIONS: ChromeActionSpec[] = [
21+
{ text: 'New folder', icon: FolderPlus },
22+
{ text: 'New base', icon: Plus, variant: 'primary' },
23+
]
2124

2225
export default function KnowledgeLoading() {
2326
return (
Lines changed: 37 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,49 @@
11
import type { QueryClient } from '@tanstack/react-query'
2+
import type { FolderApi } from '@/lib/api/contracts/folders'
23
import type { KnowledgeBaseData } from '@/lib/api/contracts/knowledge'
34
import { prefetchInternalJson } from '@/app/workspace/[workspaceId]/lib/prefetch-internal-fetch'
4-
import { KNOWLEDGE_BASE_LIST_STALE_TIME, knowledgeKeys } from '@/hooks/queries/kb/knowledge'
5+
import { FOLDER_LIST_STALE_TIME, mapFolder } from '@/hooks/queries/folders'
6+
import { KNOWLEDGE_BASE_LIST_STALE_TIME } from '@/hooks/queries/kb/knowledge'
7+
import { folderKeys } from '@/hooks/queries/utils/folder-keys'
8+
import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
59

610
/**
7-
* Prefetches the workspace's knowledge-bases list under the same query key the
8-
* client `useKnowledgeBasesQuery` hook uses (scope `active`), so the list paints
9-
* populated on first render.
11+
* Prefetches the workspace's knowledge-bases list AND its knowledge-base folder tree under
12+
* the same query keys the client `useKnowledgeBasesQuery` / `useFolders` hooks use (scope
13+
* `active`), so the list paints populated on first render.
1014
*
11-
* The list carries `Date` fields, so it goes through the `/api/knowledge` route
12-
* and caches the serialized wire shape — see {@link prefetchInternalJson}.
15+
* Both are needed: a base row is only placed correctly relative to the folder rows it sits
16+
* beside, so prefetching one without the other still flashes an ungrouped list — and a
17+
* `?folderId=` deep link renders an empty breadcrumb until the folders arrive.
18+
*
19+
* The list carries `Date` fields, so it goes through the `/api/knowledge` route and caches the
20+
* serialized wire shape — see {@link prefetchInternalJson}. Folders are mapped with the same
21+
* `mapFolder` the hook applies, so the hydrated entry matches a client fetch exactly.
1322
*/
1423
export async function prefetchKnowledgeBases(
1524
queryClient: QueryClient,
1625
workspaceId: string
1726
): Promise<void> {
18-
await queryClient.prefetchQuery({
19-
queryKey: knowledgeKeys.list(workspaceId, 'active'),
20-
queryFn: async () => {
21-
const result = await prefetchInternalJson<{ data: KnowledgeBaseData[] }>(
22-
`/api/knowledge?workspaceId=${workspaceId}&scope=active`
23-
)
24-
return result.data
25-
},
26-
staleTime: KNOWLEDGE_BASE_LIST_STALE_TIME,
27-
})
27+
await Promise.all([
28+
queryClient.prefetchQuery({
29+
queryKey: knowledgeKeys.list(workspaceId, 'active'),
30+
queryFn: async () => {
31+
const result = await prefetchInternalJson<{ data: KnowledgeBaseData[] }>(
32+
`/api/knowledge?workspaceId=${workspaceId}&scope=active`
33+
)
34+
return result.data
35+
},
36+
staleTime: KNOWLEDGE_BASE_LIST_STALE_TIME,
37+
}),
38+
queryClient.prefetchQuery({
39+
queryKey: folderKeys.list(workspaceId, 'active', 'knowledge_base'),
40+
queryFn: async () => {
41+
const { folders } = await prefetchInternalJson<{ folders?: FolderApi[] }>(
42+
`/api/folders?workspaceId=${workspaceId}&scope=active&resourceType=knowledge_base`
43+
)
44+
return (folders ?? []).map(mapFolder)
45+
},
46+
staleTime: FOLDER_LIST_STALE_TIME,
47+
}),
48+
])
2849
}

apps/sim/app/workspace/[workspaceId]/lib/prefetch.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ import { prefetchFilesBrowser } from '@/app/workspace/[workspaceId]/files/prefet
2020
import { prefetchHomeLists } from '@/app/workspace/[workspaceId]/home/prefetch'
2121
import { prefetchKnowledgeBases } from '@/app/workspace/[workspaceId]/knowledge/prefetch'
2222
import { prefetchTables } from '@/app/workspace/[workspaceId]/tables/prefetch'
23-
import { knowledgeKeys } from '@/hooks/queries/kb/knowledge'
2423
import { folderKeys } from '@/hooks/queries/utils/folder-keys'
24+
import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
2525
import { tableKeys } from '@/hooks/queries/utils/table-keys'
2626
import { workspaceFileFolderKeys } from '@/hooks/queries/workspace-file-folders'
2727
import { workspaceFilesKeys } from '@/hooks/queries/workspace-files'

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/components/sub-block/components/knowledge-base-selector/knowledge-base-selector.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ import { useSubBlockValue } from '@/app/workspace/[workspaceId]/w/[workflowId]/c
1313
import { useActiveSearchTarget } from '@/app/workspace/[workspaceId]/w/[workflowId]/components/panel/components/editor/providers/active-search-target-provider'
1414
import type { SubBlockConfig } from '@/blocks/types'
1515
import { useKnowledgeBasesList } from '@/hooks/kb/use-knowledge'
16-
import { fetchKnowledgeBase, knowledgeKeys } from '@/hooks/queries/kb/knowledge'
16+
import { fetchKnowledgeBase } from '@/hooks/queries/kb/knowledge'
17+
import { knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
1718

1819
interface KnowledgeBaseSelectorProps {
1920
blockId: string

0 commit comments

Comments
 (0)