From 74dadc5a2c48ba6f37930eaf55f5c7683bbbe4c5 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 15 Jul 2026 16:46:54 +0000 Subject: [PATCH] fix(folders): address review feedback Co-Authored-By: Paperclip --- server/src/__tests__/routines-service.test.ts | 34 +++++++++++++++ server/src/services/routines.ts | 5 +-- .../folders/FolderControls.test.tsx | 1 + ui/src/components/folders/FolderControls.tsx | 41 +++++++++++++++---- ui/src/pages/CompanySkills.tsx | 14 +++++++ ui/src/pages/Routines.tsx | 14 +++++++ 6 files changed, 97 insertions(+), 12 deletions(-) diff --git a/server/src/__tests__/routines-service.test.ts b/server/src/__tests__/routines-service.test.ts index a68955595aa..9597bdea28f 100644 --- a/server/src/__tests__/routines-service.test.ts +++ b/server/src/__tests__/routines-service.test.ts @@ -12,6 +12,7 @@ import { documentRevisions, documents, executionWorkspaces, + folders, heartbeatRuns, instanceSettings, issueInboxArchives, @@ -68,6 +69,7 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { await db.delete(routineRuns); await db.delete(routineTriggers); await db.delete(routines); + await db.delete(folders); await db.delete(routineDocuments); await db.delete(documents); await db.delete(documentRevisions); @@ -271,6 +273,38 @@ describeEmbeddedPostgres("routine service live-execution coalescing", () => { expect(allRoutines.map((entry) => entry.id)).toEqual(expect.arrayContaining([routine.id, otherRoutine.id])); }); + it("does not reveal folders owned by another company", async () => { + const { companyId, agentId, projectId, svc } = await seedFixture(); + const otherCompanyId = randomUUID(); + await db.insert(companies).values({ + id: otherCompanyId, + name: "Other company", + issuePrefix: `T${otherCompanyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + defaultResponsibleUserId: randomUUID(), + requireBoardApprovalForNewAgents: false, + }); + const [otherFolder] = await db.insert(folders).values({ + companyId: otherCompanyId, + kind: "routine", + name: "Private folder", + position: 0, + }).returning(); + + await expect(svc.create(companyId, { + projectId, + folderId: otherFolder!.id, + goalId: null, + parentIssueId: null, + title: "cross-company folder probe", + description: null, + assigneeAgentId: agentId, + priority: "medium", + status: "active", + concurrencyPolicy: "coalesce_if_active", + catchUpPolicy: "skip_missed", + }, {})).rejects.toMatchObject({ status: 404, message: "Folder not found" }); + }); + it("defaults activity gates to always at company scope", async () => { const { routine } = await seedFixture(); diff --git a/server/src/services/routines.ts b/server/src/services/routines.ts index 6a54a51930c..796cb4a11df 100644 --- a/server/src/services/routines.ts +++ b/server/src/services/routines.ts @@ -962,12 +962,11 @@ export function routineService( async function assertRoutineFolder(companyId: string, folderId: string | null | undefined) { if (!folderId) return; const folder = await db - .select({ id: folders.id, companyId: folders.companyId, kind: folders.kind }) + .select({ id: folders.id, kind: folders.kind }) .from(folders) - .where(eq(folders.id, folderId)) + .where(and(eq(folders.companyId, companyId), eq(folders.id, folderId))) .then((rows) => rows[0] ?? null); if (!folder) throw notFound("Folder not found"); - if (folder.companyId !== companyId) throw unprocessable("Folder must belong to same company"); if (folder.kind !== "routine") throw unprocessable("Folder kind must match routine"); } diff --git a/ui/src/components/folders/FolderControls.test.tsx b/ui/src/components/folders/FolderControls.test.tsx index 63b6a605400..b43523135f2 100644 --- a/ui/src/components/folders/FolderControls.test.tsx +++ b/ui/src/components/folders/FolderControls.test.tsx @@ -6,6 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { FolderListResult } from "@paperclipai/shared"; import { AllUnfiledBanner, + BulkBar, DeleteFolderDialog, FolderFormDialog, FolderRail, diff --git a/ui/src/components/folders/FolderControls.tsx b/ui/src/components/folders/FolderControls.tsx index 9b58da4c2e4..45b793bf50e 100644 --- a/ui/src/components/folders/FolderControls.tsx +++ b/ui/src/components/folders/FolderControls.tsx @@ -59,7 +59,7 @@ export const FOLDER_COLORS = [ "slate", ]; -const FOLDER_COLOR_VALUES: Record = { +const FOLDER_COLOR_VALUES: Record<(typeof FOLDER_COLORS)[number], string> = { indigo: "var(--folder-color-indigo)", violet: "var(--folder-color-violet)", emerald: "var(--folder-color-emerald)", @@ -469,6 +469,32 @@ export function MoveToMenu({ currentFolderId: string | null | undefined; onMove: (folderId: string | null) => void; onCreateAndMove: () => void; +}) { + return ( + + Move to... + + + + + ); +} + +function MoveToMenuItems({ + folders, + currentFolderId, + onMove, + onCreateAndMove, +}: { + folders: FolderListItem[]; + currentFolderId: string | null | undefined; + onMove: (folderId: string | null) => void; + onCreateAndMove: () => void; }) { const [query, setQuery] = useState(""); const visibleFolders = useMemo(() => { @@ -478,9 +504,7 @@ export function MoveToMenu({ }, [folders, query]); return ( - - Move to... - + <>
New folder... - - + ); } @@ -576,7 +599,7 @@ export function FolderFormDialog({ "h-7 w-7 rounded-md border border-border", color === swatch && "ring-2 ring-ring ring-offset-2 ring-offset-background", )} - style={{ backgroundColor: swatch }} + style={{ backgroundColor: FOLDER_COLOR_VALUES[swatch] }} onClick={() => setColor(swatch)} /> ))} @@ -671,8 +694,8 @@ export function BulkBar({ - - + { + pushToast({ + tone: "error", + title: "Folder save failed", + body: error instanceof Error ? error.message : "Failed to update folder.", + }); + }, }); const deleteFolder = useMutation({ mutationFn: (folderId: string) => foldersApi.delete(selectedCompanyId!, folderId), @@ -4383,6 +4390,13 @@ export function CompanySkills() { ]); pushToast({ tone: "success", title: "Folder deleted", body: "Skills moved to Unfiled." }); }, + onError: (error) => { + pushToast({ + tone: "error", + title: "Folder delete failed", + body: error instanceof Error ? error.message : "Failed to delete folder.", + }); + }, }); const moveSkillToFolder = useMutation({ mutationFn: ({ itemId, folderId }: { itemId: string; folderId: string | null }) => diff --git a/ui/src/pages/Routines.tsx b/ui/src/pages/Routines.tsx index 6925e2803f3..d70f1928dfb 100644 --- a/ui/src/pages/Routines.tsx +++ b/ui/src/pages/Routines.tsx @@ -466,6 +466,13 @@ export function Routines() { setFolderDialogTarget(null); await queryClient.invalidateQueries({ queryKey: queryKeys.folders.list(selectedCompanyId!, "routine") }); }, + onError: (mutationError) => { + pushToast({ + title: "Folder save failed", + body: mutationError instanceof Error ? mutationError.message : "Paperclip could not update the folder.", + tone: "error", + }); + }, }); const deleteFolder = useMutation({ mutationFn: (folderId: string) => foldersApi.delete(selectedCompanyId!, folderId), @@ -478,6 +485,13 @@ export function Routines() { ]); pushToast({ title: "Folder deleted", body: "Items moved to Unfiled.", tone: "success" }); }, + onError: (mutationError) => { + pushToast({ + title: "Folder delete failed", + body: mutationError instanceof Error ? mutationError.message : "Paperclip could not delete the folder.", + tone: "error", + }); + }, }); const moveRoutineToFolder = useMutation({ mutationFn: ({ itemId, folderId }: { itemId: string; folderId: string | null }) =>