diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index acfd253def0..720ef9435d5 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -53,11 +53,11 @@ function mockToolsList(tools: unknown[]) { } as Response); } -function createRouteApp(db: ReturnType) { +function createRouteApp(db: ReturnType, actor?: Express.Request["actor"]) { const app = express(); app.use(express.json()); app.use((req, _res, next) => { - req.actor = { + req.actor = actor ?? { type: "board", userId: "board-user", userName: "Board User", @@ -470,6 +470,90 @@ describeEmbeddedPostgres("tool access service", () => { }); }); + it("updates tool applications through the board route and records activity", async () => { + const company = await createCompany(db); + const service = toolAccessService(db); + const app = createRouteApp(db); + const application = await service.createApplication(company.id, { + name: "Editable app", + description: "Before", + type: "mcp_http", + }); + + const res = await request(app) + .patch(`/api/tool-applications/${application.id}`) + .send({ name: "Edited app", description: "After" }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + id: application.id, + companyId: company.id, + name: "Edited app", + description: "After", + type: "mcp_http", + }); + const activities = await db.select().from(activityLog).where(eq(activityLog.entityId, application.id)); + expect(activities).toEqual([ + expect.objectContaining({ + action: "tool_application.updated", + companyId: company.id, + details: expect.objectContaining({ name: "Edited app" }), + }), + ]); + }); + + it("returns 409 instead of 500 when an application update collides with a duplicate name", async () => { + const company = await createCompany(db); + const service = toolAccessService(db); + const app = createRouteApp(db); + await service.createApplication(company.id, { name: "Existing app", type: "mcp_http" }); + const application = await service.createApplication(company.id, { name: "Editable app", type: "mcp_http" }); + + const res = await request(app) + .patch(`/api/tool-applications/${application.id}`) + .send({ name: "Existing app" }); + + expect(res.status).toBe(409); + expect(res.body).toMatchObject({ + error: "A tool access record with that name already exists", + }); + }); + + it("returns 403 for cross-company application updates and 404 for missing applications", async () => { + const allowedCompany = await createCompany(db); + const otherCompany = await createCompany(db); + const application = await toolAccessService(db).createApplication(otherCompany.id, { + name: "Other company app", + type: "mcp_http", + }); + const app = createRouteApp(db, { + type: "board", + userId: "member-user", + userName: "Member User", + userEmail: null, + companyIds: [allowedCompany.id], + memberships: [ + { + companyId: allowedCompany.id, + membershipRole: "owner", + status: "active", + }, + ], + isInstanceAdmin: false, + source: "session", + }); + + const forbiddenRes = await request(app) + .patch(`/api/tool-applications/${application.id}`) + .send({ name: "Forbidden edit" }); + const missingRes = await request(createRouteApp(db)) + .patch(`/api/tool-applications/${randomUUID()}`) + .send({ name: "Missing edit" }); + + expect(forbiddenRes.status).toBe(403); + expect(missingRes.status).toBe(404); + }); + it("links run tool decisions to invocations, audit events, and pending action requests", async () => { const company = await createCompany(db); const [agent] = await db.insert(agents).values({ diff --git a/server/src/routes/tool-access.ts b/server/src/routes/tool-access.ts index 56356f0be1b..df682b1203c 100644 --- a/server/src/routes/tool-access.ts +++ b/server/src/routes/tool-access.ts @@ -127,17 +127,21 @@ export function toolAccessRoutes( assertBoard(req); const existing = await svc.getApplication(req.params.applicationId as string); assertCompanyAccess(req, existing.companyId); - const application = await svc.updateApplication(existing.id, req.body); - await logActivity(db, { - companyId: application.companyId, - actorType: "user", - actorId: req.actor.userId ?? "board", - action: "tool_application.updated", - entityType: "tool_application", - entityId: application.id, - details: { status: application.status, name: application.name }, - }); - res.json(application); + try { + const application = await svc.updateApplication(existing.id, req.body); + await logActivity(db, { + companyId: application.companyId, + actorType: "user", + actorId: req.actor.userId ?? "board", + action: "tool_application.updated", + entityType: "tool_application", + entityId: application.id, + details: { status: application.status, name: application.name }, + }); + res.json(application); + } catch (error) { + svc.ensureNoDuplicateNameError(error); + } }); router.get("/companies/:companyId/tools/connections", async (req, res) => { diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 12830908dd7..e05ea3ed126 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -2431,7 +2431,36 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }, ensureNoDuplicateNameError: (error: unknown) => { - if (error instanceof Error && /duplicate key value/.test(error.message)) { + const maybeRecord = typeof error === "object" && error !== null ? error as Record : null; + const cause = maybeRecord?.cause; + const maybeCause = typeof cause === "object" && cause !== null ? cause as Record : null; + const message = [ + error instanceof Error ? error.message : String(error), + maybeRecord && typeof maybeRecord.detail === "string" ? maybeRecord.detail : null, + maybeCause instanceof Error ? maybeCause.message : null, + maybeCause && typeof maybeCause.detail === "string" ? maybeCause.detail : null, + ].filter(Boolean).join("\n"); + const code = + maybeRecord && typeof maybeRecord.code === "string" + ? maybeRecord.code + : maybeCause && typeof maybeCause.code === "string" + ? maybeCause.code + : null; + const constraint = + maybeRecord && typeof maybeRecord.constraint === "string" + ? maybeRecord.constraint + : maybeRecord && typeof maybeRecord.constraint_name === "string" + ? maybeRecord.constraint_name + : maybeCause && typeof maybeCause.constraint === "string" + ? maybeCause.constraint + : maybeCause && typeof maybeCause.constraint_name === "string" + ? maybeCause.constraint_name + : null; + if ( + code === "23505" || + constraint?.includes("tool_applications") || + /duplicate key value|unique constraint|tool_applications_company_id_name_unique/i.test(message) + ) { throw conflict("A tool access record with that name already exists"); } throw error; diff --git a/ui/src/pages/tools/ApplicationsTab.test.tsx b/ui/src/pages/tools/ApplicationsTab.test.tsx index ca48c4cbaee..b31a1daa3e1 100644 --- a/ui/src/pages/tools/ApplicationsTab.test.tsx +++ b/ui/src/pages/tools/ApplicationsTab.test.tsx @@ -6,6 +6,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import type { CompanySecret, ToolApplication, ToolConnection } from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { ToastProvider } from "../../context/ToastContext"; +import { ApiError } from "../../api/client"; import { ApplicationsTab } from "./ApplicationsTab"; const mockToolsApi = vi.hoisted(() => ({ @@ -17,6 +18,7 @@ const mockToolsApi = vi.hoisted(() => ({ refreshCatalog: vi.fn(), createConnection: vi.fn(), updateConnection: vi.fn(), + updateApplication: vi.fn(), })); const mockSecretsApi = vi.hoisted(() => ({ @@ -48,6 +50,26 @@ async function flushReact() { }); } +async function typeInputValue(input: HTMLInputElement, value: string) { + await act(async () => { + const valueSetter = Object.getOwnPropertyDescriptor(window.HTMLInputElement.prototype, "value")?.set; + valueSetter?.call(input, value); + input.dispatchEvent(new InputEvent("input", { bubbles: true, data: value, inputType: "insertText" })); + input.dispatchEvent(new Event("change", { bubbles: true })); + }); + await flushReact(); +} + +async function typeTextareaValue(textarea: HTMLTextAreaElement, value: string) { + await act(async () => { + const valueSetter = Object.getOwnPropertyDescriptor(window.HTMLTextAreaElement.prototype, "value")?.set; + valueSetter?.call(textarea, value); + textarea.dispatchEvent(new InputEvent("input", { bubbles: true, data: value, inputType: "insertText" })); + textarea.dispatchEvent(new Event("change", { bubbles: true })); + }); + await flushReact(); +} + function makeConnection(overrides: Partial): ToolConnection { return { id: "conn-1", @@ -222,4 +244,90 @@ describe("ApplicationsTab", () => { expect(body).toContain("Credential references"); expect(body).toContain("Free-text secrets are not accepted"); }); + + it("opens the row actions edit dialog and saves application details", async () => { + mockToolsApi.updateApplication.mockResolvedValue( + makeApp({ name: "GitHub Enterprise", description: "Internal MCP" }), + ); + await render(); + + const actions = Array.from(container.querySelectorAll("button")).find( + (button) => button.getAttribute("aria-label") === "Actions for GitHub", + ); + expect(actions).toBeTruthy(); + await act(() => { + actions!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + actions!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const edit = Array.from(document.body.querySelectorAll("[role='menuitem']")).find( + (item) => item.textContent === "Edit", + ); + expect(edit).toBeTruthy(); + await act(() => { + edit!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + edit!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(document.body.textContent ?? "").toContain("Edit application"); + expect(document.body.textContent ?? "").toContain("MCP HTTP"); + + const nameInput = document.body.querySelector("#tool-application-name"); + const descriptionInput = document.body.querySelector("#tool-application-description"); + expect(nameInput).toBeTruthy(); + expect(descriptionInput).toBeTruthy(); + + await typeInputValue(nameInput!, "GitHub Enterprise"); + await typeTextareaValue(descriptionInput!, "Internal MCP"); + + const save = Array.from(document.body.querySelectorAll("button")).find((button) => + button.textContent?.includes("Save changes"), + ); + expect(save).toBeTruthy(); + await act(() => { + save!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(mockToolsApi.updateApplication).toHaveBeenCalledWith("app-1", { + name: "GitHub Enterprise", + description: "Internal MCP", + }); + }); + + it("keeps duplicate-name conflicts inline in the edit dialog", async () => { + mockToolsApi.updateApplication.mockRejectedValue(new ApiError("duplicate", 409, { error: "duplicate" })); + await render(); + + const actions = Array.from(container.querySelectorAll("button")).find( + (button) => button.getAttribute("aria-label") === "Actions for GitHub", + ); + await act(() => { + actions!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + actions!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const edit = Array.from(document.body.querySelectorAll("[role='menuitem']")).find( + (item) => item.textContent === "Edit", + ); + await act(() => { + edit!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + edit!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const save = Array.from(document.body.querySelectorAll("button")).find((button) => + button.textContent?.includes("Save changes"), + ); + await act(() => { + save!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(document.body.textContent ?? "").toContain("Another application already uses that name."); + }); }); diff --git a/ui/src/pages/tools/ApplicationsTab.tsx b/ui/src/pages/tools/ApplicationsTab.tsx index 08e7d6a1960..bf3e18fd4d5 100644 --- a/ui/src/pages/tools/ApplicationsTab.tsx +++ b/ui/src/pages/tools/ApplicationsTab.tsx @@ -5,6 +5,8 @@ import { Boxes, ChevronDown, ChevronRight, + MoreHorizontal, + Pencil, Globe, ListTree, Network, @@ -24,7 +26,22 @@ import { ApiError } from "@/api/client"; import { Button } from "@/components/ui/button"; import { Badge } from "@/components/ui/badge"; import { Card, CardContent } from "@/components/ui/card"; +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from "@/components/ui/dialog"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "@/components/ui/dropdown-menu"; import { Input } from "@/components/ui/input"; +import { Label } from "@/components/ui/label"; import { Select, SelectContent, @@ -32,6 +49,7 @@ import { SelectTrigger, SelectValue, } from "@/components/ui/select"; +import { Textarea } from "@/components/ui/textarea"; import { useToast } from "@/context/ToastContext"; import { EmptyState } from "@/components/EmptyState"; import { cn } from "@/lib/utils"; @@ -93,6 +111,78 @@ function AppIcon({ type }: { type: ToolApplicationType }) { ); } +interface EditApplicationDialogProps { + application: ToolApplication; + error: string | null; + isSaving: boolean; + onClose: () => void; + onSubmit: (input: { name: string; description: string | null }) => void; +} + +function EditApplicationDialog({ application, error, isSaving, onClose, onSubmit }: EditApplicationDialogProps) { + const [name, setName] = useState(application.name); + const [description, setDescription] = useState(application.description ?? ""); + const trimmedName = name.trim(); + + return ( + !next && onClose()}> + + + Edit application + Update the application details agents see in tool access surfaces. + +
{ + event.preventDefault(); + if (!trimmedName) return; + onSubmit({ + name: trimmedName, + description: description.trim() ? description.trim() : null, + }); + }} + > +
+ + setName(event.target.value)} + autoFocus + /> +
+
+ +