diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index a66d8059a40..cb597f54c07 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -5840,18 +5840,38 @@ describeEmbeddedPostgres("tool access service", () => { const service = toolAccessService(db); mockToolsList([ { name: "list_zaps", description: "List", inputSchema: { type: "object", properties: {} }, annotations: { readOnlyHint: true } }, + { name: "update_zap", description: "Update", inputSchema: { type: "object", properties: {} }, annotations: { readOnlyHint: false } }, ]); - const connect = await service.connectGalleryApp(company.id, { - galleryKey: "zapier", - name: "Zapier reconnect", - credentialValues: { "credentials.authorization": "old-secret" }, - }, { actorType: "user", actorId: "board" }); + const connect = await withGalleryServerUrl("zapier", PUBLIC_MCP_FIXTURE_URL, () => + service.connectGalleryApp(company.id, { + galleryKey: "zapier", + name: "Zapier reconnect", + credentialValues: { "credentials.authorization": "old-secret" }, + }, { actorType: "user", actorId: "board" })); const before = await service.getConnection(connect.connectionId, company.id); const beforeRef = before.credentialSecretRefs.find((r) => r.configPath === "credentials.authorization")!; expect(beforeRef).toBeDefined(); + const listEntry = connect.catalog.find((entry) => entry.toolName === "list_zaps")!; + const updateEntry = connect.catalog.find((entry) => entry.toolName === "update_zap")!; + const finished = await service.finishGalleryAppConnection(company.id, connect.connectionId, { + enabledCatalogEntryIds: [listEntry.id, updateEntry.id], + askFirstCatalogEntryIds: [updateEntry.id], + access: "all_agents", + }, { actorType: "user", actorId: "board" }); + await db.delete(toolProfileEntries).where(eq(toolProfileEntries.profileId, finished.profile.id)); + await db.update(toolCatalogEntries).set({ + status: "quarantined", + quarantineReason: "pending_review", + quarantinedAt: new Date(), + }).where(eq(toolCatalogEntries.connectionId, connect.connectionId)); + await db.update(toolConnections).set({ + config: { ...before.config, quarantineNewEntries: true }, + transportConfig: { ...before.transportConfig, quarantineNewEntries: true }, + }).where(eq(toolConnections.id, connect.connectionId)); + await expect( service.reconnectGalleryApp(connect.connectionId, company.id, { credentialValues: {} }, { actorType: "user", actorId: "board" }), ).rejects.toMatchObject({ message: expect.stringContaining("Paste a new key") }); @@ -5869,6 +5889,27 @@ describeEmbeddedPostgres("tool access service", () => { // Rotated in place: same secret, no duplicate ref created. expect(after.credentialSecretRefs).toHaveLength(before.credentialSecretRefs.length); expect(afterRef.secretId).toBe(beforeRef.secretId); + expect(after.config).toMatchObject({ quarantineNewEntries: false }); + expect(after.transportConfig).toMatchObject({ quarantineNewEntries: false }); + + const catalogAfterReconnect = await db.select().from(toolCatalogEntries).where( + eq(toolCatalogEntries.connectionId, connect.connectionId), + ); + expect(catalogAfterReconnect).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: listEntry.id, status: "active", quarantineReason: null }), + expect.objectContaining({ id: updateEntry.id, status: "active", quarantineReason: null }), + ])); + const profileEntriesAfterReconnect = await db.select().from(toolProfileEntries).where( + eq(toolProfileEntries.profileId, finished.profile.id), + ); + expect(profileEntriesAfterReconnect).toEqual(expect.arrayContaining([ + expect.objectContaining({ catalogEntryId: listEntry.id, effect: "include" }), + expect.objectContaining({ catalogEntryId: updateEntry.id, effect: "include" }), + ])); + await expect(db.select().from(toolPolicies).where(and( + eq(toolPolicies.companyId, company.id), + eq(toolPolicies.enabled, true), + ))).resolves.toHaveLength(0); }); it("stops and restarts local stdio runtime slots through the board service", async () => { diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 422529d9258..7fb00f1f9b7 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -4129,7 +4129,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} } } - async function refreshCatalog(connectionId: string, actor?: ActorInfo): Promise { + async function refreshCatalog( + connectionId: string, + actor?: ActorInfo, + refreshOptions: { enableAllByDefault?: boolean } = {}, + ): Promise { const connection = await getConnectionRow(connectionId); const now = new Date(); let descriptors: McpToolDescriptor[]; @@ -4161,7 +4165,8 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const sourceTemplateKey = typeof asRecord(connection.config).sourceTemplateKey === "string" ? String(asRecord(connection.config).sourceTemplateKey) : null; - const quarantineOnRefresh = shouldQuarantineNewEntries(connection) + const quarantineOnRefresh = !refreshOptions.enableAllByDefault + && shouldQuarantineNewEntries(connection) && (connection.status === "active" || sourceTemplateKey === "posthog"); const safeDefault = asRecord(connection.config).safeDefault === true; for (const descriptor of descriptors) { @@ -4239,9 +4244,17 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} } } + const normalizedConfig = refreshOptions.enableAllByDefault + ? { ...connection.config, quarantineNewEntries: false } + : connection.config; + const normalizedTransportConfig = refreshOptions.enableAllByDefault + ? { ...connection.transportConfig, quarantineNewEntries: false } + : connection.transportConfig; const [updatedConnection] = await db .update(toolConnections) .set({ + config: normalizedConfig, + transportConfig: normalizedTransportConfig, healthStatus: "ok", healthMessage: "Tool catalog refreshed.", healthCheckedAt: now, @@ -4264,15 +4277,25 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const activeEntries = updatedEntries.filter((entry) => entry.status === "active"); await enableCatalogEntriesByDefault({ connection: updatedConnection, - newCatalogEntryIds: activeEntries - .filter((entry) => { - const previous = existingByName.get(entry.toolName); - return !previous || previous.status === "quarantined"; - }) - .map((entry) => entry.id), + newCatalogEntryIds: refreshOptions.enableAllByDefault + ? activeEntries.map((entry) => entry.id) + : activeEntries + .filter((entry) => { + const previous = existingByName.get(entry.toolName); + return !previous || previous.status === "quarantined"; + }) + .map((entry) => entry.id), activeCatalogEntryIds: activeEntries.map((entry) => entry.id), actor, }); + if (refreshOptions.enableAllByDefault) { + await upsertAskFirstPolicies({ + companyId: updatedConnection.companyId, + connection: updatedConnection, + askFirstEntries: [], + actor, + }); + } await audit({ companyId: connection.companyId, @@ -6631,7 +6654,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} } throw error; } - const refresh = await refreshCatalog(connectionRow.id, actor); + const refresh = await refreshCatalog(connectionRow.id, actor, { enableAllByDefault: true }); const [application] = await db.select().from(toolApplications).where(eq(toolApplications.id, applicationRow.id)); return { connectionId: refresh.connection.id, @@ -7072,7 +7095,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} .where(eq(toolConnections.id, connection.id)) .returning(); await syncCredentialBindings(updated); - return checkConnectionHealth(updated.id, actor); + const health = await checkConnectionHealth(updated.id, actor); + const refresh = await refreshCatalog(updated.id, actor, { enableAllByDefault: true }); + return { ...health, connection: refresh.connection }; } async function startOAuth( @@ -7532,7 +7557,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} await syncCredentialBindings(connection); await checkConnectionHealth(connection.id, input.actor); - const refresh = await refreshCatalog(connection.id, input.actor); + const refresh = await refreshCatalog(connection.id, input.actor, { enableAllByDefault: true }); const [application] = await db.select().from(toolApplications).where(eq(toolApplications.id, connection.applicationId)); return { connectionId: refresh.connection.id, diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index 123d9f84371..37febf5fc00 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -388,6 +388,16 @@ describe("AppDetail", () => { expect(container.textContent).not.toContain("Action permissions"); }); + it("explains that MCP actions can take a minute while Test loads", async () => { + mockParams.tab = "test"; + listCatalogMock.mockImplementation(() => new Promise(() => undefined)); + + await renderAppDetail(); + + expect(container.textContent).toContain("Loading MCP actions, this may take a minute."); + expect(container.querySelector(".animate-spin")).toBeTruthy(); + }); + it("hides secret URL parameters in setup technical details", async () => { mockParams.tab = "setup"; getConnectionMock.mockResolvedValue( @@ -700,6 +710,35 @@ describe("AppDetail", () => { ]); }); + it("removes an existing agent grant directly from Permissions", async () => { + mockParams.tab = "permissions"; + listProfilesMock.mockResolvedValue({ + profiles: [{ + profileKey: "app:conn-1", + entries: [ + { effect: "include", catalogEntryId: "catalog-read" }, + { effect: "include", catalogEntryId: "catalog-write" }, + ], + bindings: [{ targetType: "agent", targetId: "agent-1" }], + }], + }); + + await renderAppDetail(); + + const remove = container.querySelector('button[aria-label="Remove Coder access"]'); + expect(remove).toBeTruthy(); + await act(async () => { + remove!.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(finishAppMock).toHaveBeenCalledWith("company-1", "conn-1", { + enabledCatalogEntryIds: ["catalog-read", "catalog-write"], + askFirstCatalogEntryIds: ["catalog-write"], + access: { agentIds: [] }, + }); + }); + it("renders activity attribution with issue context and human resolver names", async () => { mockParams.tab = "activity"; listConnectionActivityMock.mockResolvedValue({ diff --git a/ui/src/pages/apps/AppDetail.tsx b/ui/src/pages/apps/AppDetail.tsx index dab37d3d5dd..a981d64bc5a 100644 --- a/ui/src/pages/apps/AppDetail.tsx +++ b/ui/src/pages/apps/AppDetail.tsx @@ -520,7 +520,7 @@ export function AppDetail() { catalogQuery.isError ? { void catalogQuery.refetch(); }} /> : catalogQuery.isLoading - ? + ? : )} {activeTab === "activity" && ( @@ -636,11 +636,11 @@ function AppDetailHeader({ ); } -function ToolsLoading() { +function ToolsLoading({ mcpActions = false }: { mcpActions?: boolean }) { return (
- Loading tools… + {mcpActions ? "Loading MCP actions, this may take a minute." : "Loading tools…"}
); } diff --git a/ui/src/pages/apps/Browse.test.tsx b/ui/src/pages/apps/Browse.test.tsx index 1465cbcff23..6c40de03863 100644 --- a/ui/src/pages/apps/Browse.test.tsx +++ b/ui/src/pages/apps/Browse.test.tsx @@ -205,7 +205,7 @@ describe("Browse store door (PAP-13254 door 1)", () => { it("shows the connected owner, edits existing connections, and offers a deliberate second account", async () => { listApplicationsMock.mockResolvedValue({ applications: [ - { id: "app-notion", status: "active", applicationKey: "app-gallery:notion:one", metadata: {} }, + { id: "app-notion", name: "Legacy integration", status: "active", applicationKey: "legacy:notion", metadata: {} }, ], }); listConnectionsMock.mockResolvedValue({ @@ -216,6 +216,8 @@ describe("Browse store door (PAP-13254 door 1)", () => { name: "Notion", status: "active", createdByUserId: "user-1", + config: { sourceTemplateKey: "notion" }, + transportConfig: {}, }, { id: "conn-two", applicationId: "app-notion", status: "disabled" }, { id: "conn-draft", applicationId: "app-notion", status: "draft" }, @@ -255,7 +257,7 @@ describe("Browse store door (PAP-13254 door 1)", () => { const addAnother = container.querySelector( 'button[aria-label="Add another Notion account"]', ); - expect(addAnother?.textContent).toContain("Add another account"); + expect(addAnother?.textContent).toContain("Add new"); await act(async () => { addAnother?.dispatchEvent(new MouseEvent("click", { bubbles: true })); }); diff --git a/ui/src/pages/apps/Browse.tsx b/ui/src/pages/apps/Browse.tsx index 1898df0fb22..d0577d30277 100644 --- a/ui/src/pages/apps/Browse.tsx +++ b/ui/src/pages/apps/Browse.tsx @@ -119,6 +119,10 @@ export function Browse() { }, [gallery, trimmed]); const connectionSummaryBySlug = useMemo(() => { const connections = connectionsQuery.data?.connections ?? []; + const gallerySlugs = new Set(gallery.map((entry) => appDefinitionSlug(entry))); + const gallerySlugByName = new Map( + gallery.map((entry) => [appDefinitionName(entry).trim().toLowerCase(), appDefinitionSlug(entry)]), + ); const connectionsByApplicationId = new Map(); for (const connection of connections) { if (connection.status === "archived" || connection.status === "draft") continue; @@ -135,9 +139,17 @@ export function Browse() { }>(); for (const application of applicationsQuery.data?.applications ?? []) { if (application.status === "archived") continue; - const slug = appApplicationSourceSlug(application); - if (!slug) continue; const appConnections = connectionsByApplicationId.get(application.id) ?? []; + const configuredConnectionSlug = appConnections + .map((connection) => connection.config?.sourceTemplateKey ?? connection.transportConfig?.sourceTemplateKey) + .find((value): value is string => typeof value === "string" && gallerySlugs.has(value)); + const applicationSlug = appApplicationSourceSlug(application); + const slug = applicationSlug && gallerySlugs.has(applicationSlug) + ? applicationSlug + : configuredConnectionSlug + ?? gallerySlugByName.get(application.name.trim().toLowerCase()) + ?? null; + if (!slug) continue; const current = summaries.get(slug); summaries.set(slug, { applicationId: current?.applicationId ?? application.id, @@ -146,7 +158,7 @@ export function Browse() { }); } return summaries; - }, [applicationsQuery.data, connectionsQuery.data]); + }, [applicationsQuery.data, connectionsQuery.data, gallery]); const userProfileById = useMemo( () => buildCompanyUserProfileMap(userDirectoryQuery.data?.users), [userDirectoryQuery.data], @@ -321,7 +333,7 @@ function AppTile({ className="text-xs text-muted-foreground transition-colors hover:text-foreground" aria-label={`Add another ${appName} account`} > - Add another account + Add new )} @@ -365,7 +377,7 @@ function AppTile({ className="text-xs text-muted-foreground transition-colors hover:text-foreground" aria-label={`Add another ${appName} account`} > - Add another account + Add new )} diff --git a/ui/src/pages/apps/app-detail/PermissionsPanel.tsx b/ui/src/pages/apps/app-detail/PermissionsPanel.tsx index 90839e280a4..1307a0653f4 100644 --- a/ui/src/pages/apps/app-detail/PermissionsPanel.tsx +++ b/ui/src/pages/apps/app-detail/PermissionsPanel.tsx @@ -1,7 +1,8 @@ import { useEffect, useRef, useState } from "react"; -import { Loader2, PackageCheck, RefreshCw } from "lucide-react"; +import { Loader2, PackageCheck, RefreshCw, X } from "lucide-react"; import type { Agent, ToolCatalogEntry } from "@paperclipai/shared"; import { useSearchParams } from "@/lib/router"; +import { AgentIcon } from "@/components/AgentIconPicker"; import { Button } from "@/components/ui/button"; import { Checkbox } from "@/components/ui/checkbox"; import { AgentMultiSelect } from "@/components/AgentMultiSelect"; @@ -105,9 +106,11 @@ function AccessSection({ const summary = access.mode === "all" ? "Every agent can use it" - : `${access.agentIds.size} ${access.agentIds.size === 1 ? "agent" : "agents"} can use it`; + : access.agentIds.size === 0 + ? "No agents can use it" + : `${access.agentIds.size} ${access.agentIds.size === 1 ? "agent" : "agents"} can use it`; - const canSave = draft.mode === "all" || draft.agentIds.size > 0; + const grantedAgents = liveAgents.filter((agent) => access.agentIds.has(agent.id)); return (
@@ -123,6 +126,30 @@ function AccessSection({ )} + {!editing && access.mode === "specific" && grantedAgents.length > 0 && ( +
+ {grantedAgents.map((agent) => ( +
+ + {agent.name} + +
+ ))} +
+ )} + {editing && (