chore(folders): final polish pass — dealias imports, TSDoc comments, pinned-items test coverage

- Remove unnecessary import aliasing across the folder/pinned-items diff,
  keeping aliases only where a genuine local-identifier collision exists
- Convert route-handler summary comments to TSDoc, drop pure WHAT-restatement
  comments
- Add route + hook test coverage for /api/pinned-items (cross-workspace
  validation, unique-constraint conflict, per-user delete scoping) — the one
  area flagged by a final architecture pass as under-tested relative to risk
- Document the intentional workflow-only scope of the folder duplicate route

Author: waleedlatif1

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

@@ -17,7 +17,18 @@ import { getUserEntityPermissions } from '@/lib/workspaces/permissions/utils'
 
 const logger = createLogger('FolderDuplicateAPI')
 
-// POST /api/folders/[id]/duplicate - Duplicate a folder with all its child folders and workflows
+/**
+ * POST /api/folders/[id]/duplicate — deep-copies a workflow folder (its child
+ * folder tree and every contained workflow).
+ *
+ * This endpoint is intentionally `workflow`-only: duplication means cloning
+ * workflow *definitions*, which has no generic equivalent for file/
+ * knowledge_base/table folders yet, so — unlike the other `/api/folders`
+ * routes — it does not read `resourceType` off the row and dispatch. Passing
+ * a non-workflow folder id resolves to no source row and returns 404. When a
+ * second resource type gains a real duplicate semantic, lift this into a
+ * `performDuplicateFolder` dispatcher alongside the other orchestration fns.
+ */
 export const POST = withRouteHandler(
   async (req: NextRequest, context: { params: Promise<{ id: string }> }) => {
     const { id: sourceFolderId } = await context.params

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

@@ -1,5 +1,5 @@
 import { db } from '@sim/db'
-import { folder as folderTable } from '@sim/db/schema'
+import { folder } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { getErrorMessage } from '@sim/utils/errors'
 import { and, eq } from 'drizzle-orm'
@@ -34,9 +34,9 @@ export const POST = withRouteHandler(async (request: NextRequest, context: Route
     }
 
     const [existingFolder] = await db
-      .select({ resourceType: folderTable.resourceType })
-      .from(folderTable)
-      .where(and(eq(folderTable.id, folderId), eq(folderTable.workspaceId, workspaceId)))
+      .select({ resourceType: folder.resourceType })
+      .from(folder)
+      .where(and(eq(folder.id, folderId), eq(folder.workspaceId, workspaceId)))
       .limit(1)
 
     if (!existingFolder) {

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

@@ -1,5 +1,5 @@
 import { db } from '@sim/db'
-import { folder as folderTable } from '@sim/db/schema'
+import { folder } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { FolderLockedError } from '@sim/platform-authz/workflow'
 import { eq } from 'drizzle-orm'
@@ -44,8 +44,8 @@ export const PUT = withRouteHandler(
       // Verify the folder exists
       const existingFolder = await db
         .select()
-        .from(folderTable)
-        .where(eq(folderTable.id, id))
+        .from(folder)
+        .where(eq(folder.id, id))
         .then((rows) => rows[0])
 
       if (!existingFolder) {
@@ -136,8 +136,8 @@ export const DELETE = withRouteHandler(
       // Verify the folder exists
       const existingFolder = await db
         .select()
-        .from(folderTable)
-        .where(eq(folderTable.id, id))
+        .from(folder)
+        .where(eq(folder.id, id))
         .then((rows) => rows[0])
 
       if (!existingFolder) {

apps/sim/app/api/folders/reorder/route.ts

@@ -1,5 +1,5 @@
 import { db } from '@sim/db'
-import { folder as folderTable } from '@sim/db/schema'
+import { folder } from '@sim/db/schema'
 import { createLogger } from '@sim/logger'
 import { FolderLockedError } from '@sim/platform-authz/workflow'
 import { eq, inArray } from 'drizzle-orm'
@@ -40,12 +40,12 @@ export const PUT = withRouteHandler(async (req: NextRequest) => {
     const folderIds = updates.map((u) => u.id)
     const existingFolders = await db
       .select({
-        id: folderTable.id,
-        workspaceId: folderTable.workspaceId,
-        resourceType: folderTable.resourceType,
+        id: folder.id,
+        workspaceId: folder.workspaceId,
+        resourceType: folder.resourceType,
       })
-      .from(folderTable)
-      .where(inArray(folderTable.id, folderIds))
+      .from(folder)
+      .where(inArray(folder.id, folderIds))
 
     const validRows = existingFolders.filter((f) => f.workspaceId === workspaceId)
     const validIds = new Set(validRows.map((f) => f.id))
@@ -72,9 +72,9 @@ export const PUT = withRouteHandler(async (req: NextRequest) => {
     }
 
     const workspaceFolders = await db
-      .select({ id: folderTable.id, parentId: folderTable.parentId })
-      .from(folderTable)
-      .where(eq(folderTable.workspaceId, workspaceId))
+      .select({ id: folder.id, parentId: folder.parentId })
+      .from(folder)
+      .where(eq(folder.workspaceId, workspaceId))
 
     const parentById = new Map<string, string | null>()
     for (const folderRow of workspaceFolders) {

apps/sim/app/api/pinned-items/[resourceType]/[resourceId]/route.test.ts

@@ -0,0 +1,139 @@
+/**
+ * Tests for the pinned-item delete route (/api/pinned-items/[resourceType]/[resourceId])
+ *
+ * @vitest-environment node
+ */
+import { authMockFns, createMockRequest, schemaMock } from '@sim/testing'
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+const { mockLogger, mockDb } = vi.hoisted(() => {
+  const logger = {
+    info: vi.fn(),
+    warn: vi.fn(),
+    error: vi.fn(),
+    debug: vi.fn(),
+    trace: vi.fn(),
+    fatal: vi.fn(),
+    child: vi.fn(),
+  }
+  return {
+    mockLogger: logger,
+    mockDb: { delete: vi.fn() },
+  }
+})
+
+vi.mock('@sim/logger', () => ({
+  createLogger: vi.fn().mockReturnValue(mockLogger),
+  runWithRequestContext: <T>(_ctx: unknown, fn: () => T): T => fn(),
+  getRequestContext: () => undefined,
+}))
+// The route imports `pinnedItem` (a table schema object) from `@sim/db`
+// alongside `db` — merge in `schemaMock`'s table shapes like the sibling
+// pinned-items/route.test.ts does.
+vi.mock('@sim/db', () => ({ db: mockDb, ...schemaMock }))
+
+import { DELETE } from '@/app/api/pinned-items/[resourceType]/[resourceId]/route'
+
+const OWNER_USER = { id: 'user-123', email: 'owner@example.com', name: 'Owner User' }
+const OTHER_USER = { id: 'user-456', email: 'other@example.com', name: 'Other User' }
+
+/** One pinned row: `resourceType`='workflow', `resourceId`='workflow-1', owned by OWNER_USER. */
+const ownerPinnedRow = { id: 'pinned-1' }
+
+function makeParams(resourceType: string, resourceId: string) {
+  return Promise.resolve({ resourceType, resourceId })
+}
+
+describe('Pinned Item Delete Route', () => {
+  const mockDelete = mockDb.delete
+  const mockWhere = vi.fn()
+  const mockReturning = vi.fn()
+
+  function mockAuthenticatedUser(user: typeof OWNER_USER = OWNER_USER) {
+    authMockFns.mockGetSession.mockResolvedValue({ user })
+  }
+
+  function mockUnauthenticated() {
+    authMockFns.mockGetSession.mockResolvedValue(null)
+  }
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+
+    mockDelete.mockReturnValue({ where: mockWhere })
+    mockWhere.mockReturnValue({ returning: mockReturning })
+    mockReturning.mockReturnValue([])
+  })
+
+  it('should unpin a resource successfully', async () => {
+    mockAuthenticatedUser(OWNER_USER)
+    mockReturning.mockReturnValueOnce([ownerPinnedRow])
+
+    const req = createMockRequest('DELETE')
+    const response = await DELETE(req, { params: makeParams('workflow', 'workflow-1') })
+
+    expect(response.status).toBe(200)
+    const data = await response.json()
+    expect(data).toHaveProperty('success', true)
+    expect(mockLogger.info).toHaveBeenCalledWith('Unpinned resource', {
+      resourceType: 'workflow',
+      resourceId: 'workflow-1',
+      userId: OWNER_USER.id,
+    })
+  })
+
+  it('should scope the delete by the caller userId, not by workspace, and never delete another user pin of the same resource', async () => {
+    // Simulate the query condition by asserting the delete is filtered with the
+    // requesting user's id, and that a mismatched-owner deletion yields zero rows.
+    mockAuthenticatedUser(OTHER_USER)
+    // Because the composite where() is keyed on (userId, resourceType, resourceId),
+    // OTHER_USER's delete of OWNER_USER's pin matches nothing -> empty returning().
+    mockReturning.mockReturnValueOnce([])
+
+    const req = createMockRequest('DELETE')
+    const response = await DELETE(req, { params: makeParams('workflow', 'workflow-1') })
+
+    expect(response.status).toBe(404)
+    const data = await response.json()
+    expect(data).toHaveProperty('error', 'Pinned item not found')
+
+    // The delete must always be scoped to the authenticated caller's own userId.
+    const { eq } = await import('drizzle-orm')
+    expect(eq).toHaveBeenCalledWith(expect.anything(), OTHER_USER.id)
+    expect(mockLogger.info).not.toHaveBeenCalled()
+  })
+
+  it('should return 401 for unauthenticated requests', async () => {
+    mockUnauthenticated()
+
+    const req = createMockRequest('DELETE')
+    const response = await DELETE(req, { params: makeParams('workflow', 'workflow-1') })
+
+    expect(response.status).toBe(401)
+    const data = await response.json()
+    expect(data).toHaveProperty('error', 'Unauthorized')
+    expect(mockDelete).not.toHaveBeenCalled()
+  })
+
+  it('should return 404 when the pin does not exist', async () => {
+    mockAuthenticatedUser(OWNER_USER)
+    mockReturning.mockReturnValueOnce([])
+
+    const req = createMockRequest('DELETE')
+    const response = await DELETE(req, { params: makeParams('workflow', 'workflow-missing') })
+
+    expect(response.status).toBe(404)
+    const data = await response.json()
+    expect(data).toHaveProperty('error', 'Pinned item not found')
+  })
+
+  it('should return 400 for an invalid resourceType path segment', async () => {
+    mockAuthenticatedUser(OWNER_USER)
+
+    const req = createMockRequest('DELETE')
+    const response = await DELETE(req, { params: makeParams('not-a-real-type', 'workflow-1') })
+
+    expect(response.status).toBe(400)
+    expect(mockDelete).not.toHaveBeenCalled()
+  })
+})

apps/sim/app/api/pinned-items/[resourceType]/[resourceId]/route.ts

@@ -11,7 +11,7 @@ const logger = createLogger('PinnedItemDeleteAPI')
 
 type RouteContext = { params: Promise<{ resourceType: string; resourceId: string }> }
 
-// DELETE - Unpin a resource, keyed by the composite (resourceType, resourceId)
+/** Unpins a resource, identified by the composite key (`resourceType`, `resourceId`). */
 export const DELETE = withRouteHandler(async (request: NextRequest, context: RouteContext) => {
   const session = await getSession()
   if (!session?.user?.id) {