From 6f2490afbee14dd63ee1b46b6b9a92d536fbc909 Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 10 Jun 2026 16:06:01 +0000 Subject: [PATCH] Add guarded application delete (Phase 4, PAP-10817) DELETE /tool-applications/:applicationId with board-only auth + company check mirroring the PATCH route. Guards with 409 when the application still has connections (no force-cascade in v1) and writes a tool_application.deleted activity log entry. Adds deleteApplication to the UI client and a destructive confirm dialog in the row actions menu that surfaces the connection-guard guidance inline on 409. Co-Authored-By: Claude Opus 4.8 --- packages/shared/src/types/tool-access.ts | 1 + .../tool-access-policy-service.test.ts | 45 ++++ .../src/__tests__/tool-access-service.test.ts | 85 ++++++ server/src/routes/tool-access.ts | 17 ++ server/src/services/tool-access-policy.ts | 3 + server/src/services/tool-access.ts | 20 ++ ui/src/api/tools.ts | 2 + ui/src/pages/tools/ApplicationsTab.test.tsx | 158 +++++++++++ ui/src/pages/tools/ApplicationsTab.tsx | 254 +++++++++++++++++- 9 files changed, 582 insertions(+), 3 deletions(-) diff --git a/packages/shared/src/types/tool-access.ts b/packages/shared/src/types/tool-access.ts index 5ef3d22dc1c..80e895af5c9 100644 --- a/packages/shared/src/types/tool-access.ts +++ b/packages/shared/src/types/tool-access.ts @@ -562,6 +562,7 @@ export type ToolAccessReasonCode = | "deny_default" | "deny_company_boundary" | "deny_disabled_connection" + | "deny_disabled_application" | "deny_archived_application" | "deny_missing_tool" | "deny_policy_block" diff --git a/server/src/__tests__/tool-access-policy-service.test.ts b/server/src/__tests__/tool-access-policy-service.test.ts index a1ae6a4bfda..4e5ed835e5a 100644 --- a/server/src/__tests__/tool-access-policy-service.test.ts +++ b/server/src/__tests__/tool-access-policy-service.test.ts @@ -238,6 +238,51 @@ describeEmbeddedPostgres("tool access policy service", () => { }); }); + it("denies calls through disabled applications before explicit grants and allows them after reactivation", async () => { + const company = await createCompany(db); + const agent = await createAgent(db, company.id); + const { application, connection, catalogEntry } = await createTool(db, company.id); + await db.insert(principalPermissionGrants).values({ + companyId: company.id, + principalType: "agent", + principalId: agent.id, + permissionKey: "tools:use", + scope: { toolName: "send_email" }, + }); + const input = { + companyId: company.id, + actor: { actorType: "agent" as const, actorId: agent.id, agentId: agent.id }, + request: { connectionId: connection.id, catalogEntryId: catalogEntry.id, toolName: "send_email" }, + }; + + await expect(toolAccessPolicyService(db).decide(input)).resolves.toMatchObject({ + allowed: true, + decision: "allow", + reasonCode: "allow_explicit_grant", + }); + + await db + .update(toolApplications) + .set({ status: "disabled", updatedAt: new Date() }) + .where(eq(toolApplications.id, application.id)); + await expect(toolAccessPolicyService(db).decide(input)).resolves.toMatchObject({ + allowed: false, + decision: "deny", + reasonCode: "deny_disabled_application", + explanation: "Application is disabled.", + }); + + await db + .update(toolApplications) + .set({ status: "active", updatedAt: new Date() }) + .where(eq(toolApplications.id, application.id)); + await expect(toolAccessPolicyService(db).decide(input)).resolves.toMatchObject({ + allowed: true, + decision: "allow", + reasonCode: "allow_explicit_grant", + }); + }); + it("manages generic tool policies without exposing trust rules", async () => { const company = await createCompany(db); const otherCompany = await createCompany(db); diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 720ef9435d5..db262604047 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -554,6 +554,91 @@ describeEmbeddedPostgres("tool access service", () => { expect(missingRes.status).toBe(404); }); + it("deletes an application with zero connections 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: "Deletable app", + type: "mcp_http", + }); + + const res = await request(app).delete(`/api/tool-applications/${application.id}`); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ id: application.id, name: "Deletable app" }); + const remaining = await db + .select() + .from(toolApplications) + .where(eq(toolApplications.id, application.id)); + expect(remaining).toHaveLength(0); + const activities = await db.select().from(activityLog).where(eq(activityLog.entityId, application.id)); + expect(activities).toEqual([ + expect.objectContaining({ + action: "tool_application.deleted", + companyId: company.id, + details: expect.objectContaining({ name: "Deletable app", type: "mcp_http" }), + }), + ]); + }); + + it("returns 409 and keeps the application when it still has connections", async () => { + const company = await createCompany(db); + const service = toolAccessService(db); + const app = createRouteApp(db); + const connection = await service.createConnection(company.id, { + name: "Guarded connection", + transport: "remote_http", + config: { url: "https://fixture.example/mcp" }, + }); + + const res = await request(app).delete(`/api/tool-applications/${connection.applicationId}`); + + expect(res.status).toBe(409); + expect(String(res.body.error)).toMatch(/connection/i); + const remaining = await db + .select() + .from(toolApplications) + .where(eq(toolApplications.id, connection.applicationId)); + expect(remaining).toHaveLength(1); + }); + + it("returns 403 for cross-company application deletes 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).delete(`/api/tool-applications/${application.id}`); + const missingRes = await request(createRouteApp(db)).delete(`/api/tool-applications/${randomUUID()}`); + + expect(forbiddenRes.status).toBe(403); + expect(missingRes.status).toBe(404); + const stillThere = await db + .select() + .from(toolApplications) + .where(eq(toolApplications.id, application.id)); + expect(stillThere).toHaveLength(1); + }); + 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 df682b1203c..fca7f483444 100644 --- a/server/src/routes/tool-access.ts +++ b/server/src/routes/tool-access.ts @@ -144,6 +144,23 @@ export function toolAccessRoutes( } }); + router.delete("/tool-applications/:applicationId", async (req, res) => { + assertBoard(req); + const existing = await svc.getApplication(req.params.applicationId as string); + assertCompanyAccess(req, existing.companyId); + const application = await svc.deleteApplication(existing.id); + await logActivity(db, { + companyId: application.companyId, + actorType: "user", + actorId: req.actor.userId ?? "board", + action: "tool_application.deleted", + entityType: "tool_application", + entityId: application.id, + details: { type: application.type, name: application.name }, + }); + res.json(application); + }); + router.get("/companies/:companyId/tools/connections", async (req, res) => { assertBoard(req); const companyId = req.params.companyId as string; diff --git a/server/src/services/tool-access-policy.ts b/server/src/services/tool-access-policy.ts index 49625f50425..3840e6dc0dd 100644 --- a/server/src/services/tool-access-policy.ts +++ b/server/src/services/tool-access-policy.ts @@ -546,6 +546,9 @@ export function toolAccessPolicyService(db: Db) { if (!application || application.companyId !== input.companyId) { return { ok: false, redaction, decision: decision("deny", "deny_company_boundary", "Application is outside the company.", [], [], { redactionPlan: redaction.redactionPlan }) }; } + if (application.status === "disabled") { + return { ok: false, redaction, decision: decision("deny", "deny_disabled_application", "Application is disabled.", [], [], { redactionPlan: redaction.redactionPlan }) }; + } if (application.status === "archived") { return { ok: false, redaction, decision: decision("deny", "deny_archived_application", "Application is archived.", [], [], { redactionPlan: redaction.redactionPlan }) }; } diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index d247d213d59..de209cd0d66 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -1886,6 +1886,26 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} return toApplication(row); }, + deleteApplication: async (applicationId: string): Promise => { + const [existing] = await db.select().from(toolApplications).where(eq(toolApplications.id, applicationId)); + if (!existing) throw notFound("Tool application not found"); + // Guard: never orphan connections. The caller must remove the connections + // or archive the application instead — there is no force-cascade in v1. + const linkedConnections = await db + .select({ id: toolConnections.id }) + .from(toolConnections) + .where(eq(toolConnections.applicationId, applicationId)); + if (linkedConnections.length > 0) { + throw conflict( + "This application still has connections. Remove its connections or archive the application instead of deleting it.", + { connectionCount: linkedConnections.length }, + ); + } + const [row] = await db.delete(toolApplications).where(eq(toolApplications.id, applicationId)).returning(); + if (!row) throw notFound("Tool application not found"); + return toApplication(row); + }, + listConnections: async (companyId: string): Promise => { const rows = await db .select() diff --git a/ui/src/api/tools.ts b/ui/src/api/tools.ts index 8a00d882a90..1e4e639e280 100644 --- a/ui/src/api/tools.ts +++ b/ui/src/api/tools.ts @@ -165,6 +165,8 @@ export const toolsApi = { api.post(`/companies/${companyId}/tools/applications`, input), updateApplication: (applicationId: string, input: UpdateToolApplicationInput) => api.patch(`/tool-applications/${applicationId}`, input), + deleteApplication: (applicationId: string) => + api.delete(`/tool-applications/${applicationId}`), // --- Connections --- listConnections: (companyId: string) => diff --git a/ui/src/pages/tools/ApplicationsTab.test.tsx b/ui/src/pages/tools/ApplicationsTab.test.tsx index b31a1daa3e1..ad63f9435ef 100644 --- a/ui/src/pages/tools/ApplicationsTab.test.tsx +++ b/ui/src/pages/tools/ApplicationsTab.test.tsx @@ -19,6 +19,7 @@ const mockToolsApi = vi.hoisted(() => ({ createConnection: vi.fn(), updateConnection: vi.fn(), updateApplication: vi.fn(), + deleteApplication: vi.fn(), })); const mockSecretsApi = vi.hoisted(() => ({ @@ -330,4 +331,161 @@ describe("ApplicationsTab", () => { expect(document.body.textContent ?? "").toContain("Another application already uses that name."); }); + + it("confirms disabling an application with connection and catalog impact", async () => { + mockToolsApi.updateApplication.mockResolvedValue(makeApp({ status: "disabled" })); + 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 disable = Array.from(document.body.querySelectorAll("[role='menuitem']")).find( + (item) => item.textContent === "Disable", + ); + expect(disable).toBeTruthy(); + await act(() => { + disable!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + disable!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const body = document.body.textContent ?? ""; + expect(body).toContain("Disable application"); + expect(body).toContain("Impact summary"); + expect(body).toContain("1connection affected"); + expect(body).toContain("2catalog tools affected"); + + const confirm = Array.from(document.body.querySelectorAll("button")).find((button) => + button.textContent?.includes("Disable application"), + ); + expect(confirm).toBeTruthy(); + await act(() => { + confirm!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(mockToolsApi.updateApplication).toHaveBeenCalledWith("app-1", { status: "disabled" }); + }); + + it("reactivates disabled applications directly from the row actions menu", async () => { + mockToolsApi.listApplications.mockResolvedValue({ + applications: [makeApp({ status: "disabled" }), makeApp({ id: "app-2", name: "Linear", description: null })], + }); + mockToolsApi.updateApplication.mockResolvedValue(makeApp({ status: "active" })); + 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 reactivate = Array.from(document.body.querySelectorAll("[role='menuitem']")).find( + (item) => item.textContent === "Reactivate", + ); + expect(reactivate).toBeTruthy(); + await act(() => { + reactivate!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + reactivate!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(mockToolsApi.updateApplication).toHaveBeenCalledWith("app-1", { status: "active" }); + }); + + it("deletes an application without connections through the row actions menu", async () => { + mockToolsApi.deleteApplication.mockResolvedValue(makeApp({ id: "app-2", name: "Linear", description: null })); + await render(); + + const actions = Array.from(container.querySelectorAll("button")).find( + (button) => button.getAttribute("aria-label") === "Actions for Linear", + ); + expect(actions).toBeTruthy(); + await act(() => { + actions!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + actions!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const del = Array.from(document.body.querySelectorAll("[role='menuitem']")).find( + (item) => item.textContent === "Delete", + ); + expect(del).toBeTruthy(); + await act(() => { + del!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + del!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const body = document.body.textContent ?? ""; + expect(body).toContain("Delete application"); + expect(body).toContain("No connections are attached"); + + const confirm = Array.from(document.body.querySelectorAll("button")).find((button) => + button.textContent?.includes("Delete application"), + ); + expect(confirm).toBeTruthy(); + await act(() => { + confirm!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(mockToolsApi.deleteApplication).toHaveBeenCalledWith("app-2"); + }); + + it("surfaces the connection guard message inline when delete returns 409", async () => { + mockToolsApi.deleteApplication.mockRejectedValue( + new ApiError( + "This application still has connections. Remove its connections or archive the application instead of deleting it.", + 409, + { error: "conflict" }, + ), + ); + 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 del = Array.from(document.body.querySelectorAll("[role='menuitem']")).find( + (item) => item.textContent === "Delete", + ); + await act(() => { + del!.dispatchEvent(new PointerEvent("pointerdown", { bubbles: true })); + del!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + // GitHub (app-1) has a connection, so the dialog warns before we even submit. + expect(document.body.textContent ?? "").toContain("delete is blocked while connections exist"); + + const confirm = Array.from(document.body.querySelectorAll("button")).find((button) => + button.textContent?.includes("Delete application"), + ); + await act(() => { + confirm!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(mockToolsApi.deleteApplication).toHaveBeenCalledWith("app-1"); + expect(document.body.textContent ?? "").toContain( + "Remove its connections or archive the application instead", + ); + }); }); diff --git a/ui/src/pages/tools/ApplicationsTab.tsx b/ui/src/pages/tools/ApplicationsTab.tsx index bf3e18fd4d5..c7983063cd4 100644 --- a/ui/src/pages/tools/ApplicationsTab.tsx +++ b/ui/src/pages/tools/ApplicationsTab.tsx @@ -2,6 +2,7 @@ import { Fragment, useMemo, useState } from "react"; import { useMutation, useQueries, useQuery, useQueryClient } from "@tanstack/react-query"; import { AppWindow, + Archive, Boxes, ChevronDown, ChevronRight, @@ -14,8 +15,10 @@ import { Plus, Power, RefreshCw, + RotateCcw, Stethoscope, Terminal, + Trash2, Upload, type LucideIcon, } from "lucide-react"; @@ -38,6 +41,7 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, + DropdownMenuSeparator, DropdownMenuTrigger, } from "@/components/ui/dropdown-menu"; import { Input } from "@/components/ui/input"; @@ -183,6 +187,117 @@ function EditApplicationDialog({ application, error, isSaving, onClose, onSubmit ); } +type ApplicationLifecycleAction = { + application: ToolApplication; + status: "disabled" | "archived"; + connectionCount: number; + toolCount: number; +}; + +function LifecycleConfirmDialog({ + action, + isSaving, + onClose, + onConfirm, +}: { + action: ApplicationLifecycleAction; + isSaving: boolean; + onClose: () => void; + onConfirm: () => void; +}) { + const verb = action.status === "archived" ? "Archive" : "Disable"; + const lower = verb.toLowerCase(); + const pendingLabel = action.status === "archived" ? "Archiving..." : "Disabling..."; + const connectionLabel = action.connectionCount === 1 ? "connection" : "connections"; + const toolLabel = action.toolCount === 1 ? "catalog tool" : "catalog tools"; + + return ( + !next && onClose()}> + + + {verb} application + + {lower === "archive" ? "Archive" : "Disable"} {action.application.name} for every connection attached to it. + + +
+
Impact summary
+
+
+
{action.connectionCount}
+
{connectionLabel} affected
+
+
+
{action.toolCount}
+
{toolLabel} affected
+
+
+
+ + + + + + + ); +} + +function DeleteApplicationDialog({ + application, + connectionCount, + error, + isDeleting, + onClose, + onConfirm, +}: { + application: ToolApplication; + connectionCount: number; + error: string | null; + isDeleting: boolean; + onClose: () => void; + onConfirm: () => void; +}) { + return ( + !next && onClose()}> + + + Delete application + + Permanently delete {application.name}. This cannot be undone. + + + {connectionCount > 0 ? ( +
+ This application still has {connectionCount} {connectionCount === 1 ? "connection" : "connections"}. Remove + them or archive the application instead — delete is blocked while connections exist. +
+ ) : ( +
+ No connections are attached, so this application can be deleted safely. +
+ )} + {error ? ( +
+ {error} +
+ ) : null} + + + + + + + ); +} + export function ApplicationsTab({ companyId }: { companyId: string }) { const qc = useQueryClient(); const { pushToast } = useToast(); @@ -192,6 +307,9 @@ export function ApplicationsTab({ companyId }: { companyId: string }) { const [catalogFor, setCatalogFor] = useState(null); const [editingApplication, setEditingApplication] = useState(null); const [editError, setEditError] = useState(null); + const [lifecycleAction, setLifecycleAction] = useState(null); + const [deletingApplication, setDeletingApplication] = useState(null); + const [deleteError, setDeleteError] = useState(null); const [search, setSearch] = useState(""); const [typeFilter, setTypeFilter] = useState("__all"); @@ -253,13 +371,24 @@ export function ApplicationsTab({ companyId }: { companyId: string }) { const invalidateApplications = () => qc.invalidateQueries({ queryKey: queryKeys.tools.applications(companyId) }); const updateApplication = useMutation({ - mutationFn: ({ applicationId, input }: { applicationId: string; input: { name: string; description: string | null } }) => + mutationFn: ({ + applicationId, + input, + }: { + applicationId: string; + input: { name?: string; description?: string | null; status?: ToolApplication["status"] }; + }) => toolsApi.updateApplication(applicationId, input), - onSuccess: () => { + onSuccess: (application, variables) => { invalidateApplications(); setEditingApplication(null); + setLifecycleAction(null); setEditError(null); - pushToast({ title: "Application updated", tone: "success" }); + const status = variables.input.status; + pushToast({ + title: status ? `Application ${status === "active" ? "reactivated" : status}` : "Application updated", + tone: "success", + }); }, onError: (err) => { const message = err instanceof ApiError ? err.message : String(err); @@ -276,6 +405,27 @@ export function ApplicationsTab({ companyId }: { companyId: string }) { }, }); + const deleteApplication = useMutation({ + mutationFn: (applicationId: string) => toolsApi.deleteApplication(applicationId), + onSuccess: (application) => { + invalidateApplications(); + invalidateConnections(); + setDeletingApplication(null); + setDeleteError(null); + pushToast({ title: `Deleted ${application.name}`, tone: "success" }); + }, + onError: (err) => { + // The server returns 409 when the application still has connections; show + // the "remove connections or archive instead" guidance inline rather than + // as a transient toast. + const message = err instanceof ApiError ? err.message : String(err); + setDeleteError(message); + if (!(err instanceof ApiError && err.status === 409)) { + pushToast({ title: "Could not delete application", body: message, tone: "error" }); + } + }, + }); + const healthCheck = useMutation({ mutationFn: (id: string) => toolsApi.checkConnectionHealth(id), onSuccess: (res) => { @@ -512,6 +662,71 @@ export function ApplicationsTab({ companyId }: { companyId: string }) { Edit + {app.status !== "archived" ? ( + <> + + {app.status !== "disabled" ? ( + + setLifecycleAction({ + application: app, + status: "disabled", + connectionCount: connCountByApp.get(app.id) ?? 0, + toolCount: toolCountByApp.get(app.id) ?? 0, + }) + } + > + + Disable + + ) : null} + + setLifecycleAction({ + application: app, + status: "archived", + connectionCount: connCountByApp.get(app.id) ?? 0, + toolCount: toolCountByApp.get(app.id) ?? 0, + }) + } + > + + Archive + + + ) : null} + {app.status === "disabled" || app.status === "archived" ? ( + <> + + + updateApplication.mutate({ + applicationId: app.id, + input: { status: "active" }, + }) + } + > + + Reactivate + + + ) : null} + + { + setDeleteError(null); + setDeletingApplication(app); + }} + > + + Delete + @@ -651,6 +866,39 @@ export function ApplicationsTab({ companyId }: { companyId: string }) { }} /> ) : null} + {lifecycleAction ? ( + { + if (updateApplication.isPending) return; + setLifecycleAction(null); + }} + onConfirm={() => + updateApplication.mutate({ + applicationId: lifecycleAction.application.id, + input: { status: lifecycleAction.status }, + }) + } + /> + ) : null} + {deletingApplication ? ( + { + if (deleteApplication.isPending) return; + setDeletingApplication(null); + setDeleteError(null); + }} + onConfirm={() => { + setDeleteError(null); + deleteApplication.mutate(deletingApplication.id); + }} + /> + ) : null} {open ? (