diff --git a/packages/shared/src/types/tool-access.ts b/packages/shared/src/types/tool-access.ts index b3d6e9001ed..6bbcffcc69f 100644 --- a/packages/shared/src/types/tool-access.ts +++ b/packages/shared/src/types/tool-access.ts @@ -1411,6 +1411,8 @@ export interface ToolConnectionTestAgent { role: string; title: string | null; status: string; + /** Zero-based depth in the company reporting tree; roots are highest-ranked. */ + orgDepth: number; effectiveAccess: ToolConnectionAccessSummary; } diff --git a/server/src/__tests__/generic-mcp-connection.test.ts b/server/src/__tests__/generic-mcp-connection.test.ts index d278796d5db..0d53ea0a541 100644 --- a/server/src/__tests__/generic-mcp-connection.test.ts +++ b/server/src/__tests__/generic-mcp-connection.test.ts @@ -344,7 +344,7 @@ describeEmbeddedPostgres("generic remote MCP connections", () => { await tempDb?.cleanup(); }); - it("connects a public unknown endpoint with conservative defaults and quarantine", async () => { + it("connects a public unknown endpoint with every discovered tool enabled", async () => { installMcpOAuthFixture({ auth: "public" }); const company = await createCompany(db); const service = toolAccessService(db); @@ -354,18 +354,30 @@ describeEmbeddedPostgres("generic remote MCP connections", () => { expect(result.auth ?? null).toBeNull(); expect(result.actions.readOnly.map((action) => action.toolName)).toEqual(["list_insights"]); expect(result.actions.canMakeChanges.map((action) => action.toolName)).toEqual(["create_insight"]); - // Reads are on for review, anything that can change data starts off. expect(result.suggestedDefaults).toMatchObject({ askFirstRiskLevels: ["write", "destructive"] }); const [connection] = await db.select().from(toolConnections).where(eq(toolConnections.id, result.connectionId)); expect(connection).toMatchObject({ transport: "mcp_remote", authKind: "none", status: "draft" }); - // An unknown endpoint stays labelled unverified and quarantines whatever it - // grows later, exactly like the curated path's conservative defaults. - expect(connection!.config).toMatchObject({ quarantineNewEntries: true, unverifiedServer: true }); + expect(connection!.config).toMatchObject({ quarantineNewEntries: false, unverifiedServer: true }); // No curated definition was consulted: nothing recorded a template key, so // this connection cannot be depending on gallery metadata for anything. expect(connection!.config).not.toHaveProperty("sourceTemplateKey"); expect(connection!.config).not.toHaveProperty("connectionMethodKey"); + const [profile] = await db.select().from(toolProfiles).where(eq( + toolProfiles.profileKey, + `app:${result.connectionId}`, + )); + expect(profile).toBeTruthy(); + await expect(db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + profile!.id, + ))).resolves.toHaveLength(2); + await expect(db.select().from(toolProfileBindings).where(eq( + toolProfileBindings.profileId, + profile!.id, + ))).resolves.toEqual([ + expect.objectContaining({ targetType: "company", targetId: company.id }), + ]); }); it("emits DNS guidance for a real NXDOMAIN failure", async () => { diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 86fd66cfcbf..a66d8059a40 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -99,6 +99,24 @@ function mockToolsList(tools: unknown[]) { ); } +const PUBLIC_MCP_FIXTURE_URL = "https://8.8.8.8/api/mcp"; + +async function withGalleryServerUrl( + slug: string, + serverUrl: string, + operation: () => Promise, +): Promise { + const method = getConnectableAppDefinition(slug)?.methods[0]; + if (!method?.defaults) throw new Error(`Missing gallery method defaults for ${slug}`); + const originalServerUrl = method.defaults.serverUrl; + method.defaults.serverUrl = serverUrl; + try { + return await operation(); + } finally { + method.defaults.serverUrl = originalServerUrl; + } +} + function createRouteApp( db: ReturnType, actor?: Express.Request["actor"], @@ -147,13 +165,14 @@ async function grantBoardUser( companyId: string, userId: string, permissionKeys: string[], + membershipRole: "owner" | "admin" | "operator" | "member" | "viewer" = "operator", ) { await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId, status: "active", - membershipRole: "operator", + membershipRole, }); if (permissionKeys.length > 0) { await db.insert(principalPermissionGrants).values(permissionKeys.map((permissionKey) => ({ @@ -1447,6 +1466,7 @@ describeEmbeddedPostgres("tool access service", () => { expect(res.body.agents).toHaveLength(1); expect(res.body.agents[0]).toMatchObject({ id: agent.id, + orgDepth: 0, effectiveAccess: { connectionId: connection.id, toolCount: 1, @@ -1457,6 +1477,55 @@ describeEmbeddedPostgres("tool access service", () => { }); }); + it("lists only writable agents and ranks the highest accessible agent first", async () => { + const company = await createCompany(db); + const userId = `scoped-tool-tester-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, ["tools:use"], "viewer"); + const actor = boardSessionActor(company.id, "viewer", userId); + const root = await createAgent(db, company.id); + const [accessibleManager] = await db.insert(agents).values({ + companyId: company.id, + name: "Accessible manager", + role: "manager", + reportsTo: root.id, + status: "active", + adapterType: "process", + adapterConfig: {}, + runtimeConfig: {}, + }).returning(); + const [accessibleReport] = await db.insert(agents).values({ + companyId: company.id, + name: "Accessible report", + role: "engineer", + reportsTo: accessibleManager!.id, + status: "active", + adapterType: "process", + adapterConfig: {}, + runtimeConfig: {}, + }).returning(); + await db.insert(principalPermissionGrants).values({ + companyId: company.id, + principalType: "user", + principalId: userId, + permissionKey: "tasks:assign_scope", + scope: { agentIds: [accessibleManager!.id, accessibleReport!.id] }, + grantedByUserId: "owner", + }); + const { connection } = await createRemoteToolFixture(db, company.id); + const app = createRouteApp(db, actor, createToolGatewayService(db, { toolActionSigningSecret: "test-secret" })); + + const res = await request(app) + .get(`/api/tool-connections/${connection.id}/test-agents`) + .expect(200); + + expect(res.body.agents.map((agent: { id: string }) => agent.id)).toEqual([ + accessibleManager!.id, + accessibleReport!.id, + ]); + expect(res.body.agents.map((agent: { orgDepth: number }) => agent.orgDepth)).toEqual([1, 2]); + expect(res.body.agents).not.toEqual(expect.arrayContaining([expect.objectContaining({ id: root.id })])); + }); + it("surfaces a last-changed audit hint attributed to the agent that authored the governing policy", async () => { const company = await createCompany(db); const userId = `tool-tester-${randomUUID()}`; @@ -5295,15 +5364,16 @@ describeEmbeddedPostgres("tool access service", () => { runtimeConfig: {}, }).returning(); - const connect = await service.connectGalleryApp(company.id, { - galleryKey: "zapier", - name: "Zapier workspace", - credentialValues: { "credentials.authorization": "zap-secret" }, - }, { actorType: "user", actorId: "board" }); + const connect = await withGalleryServerUrl("zapier", PUBLIC_MCP_FIXTURE_URL, () => + service.connectGalleryApp(company.id, { + galleryKey: "zapier", + name: "Zapier workspace", + credentialValues: { "credentials.authorization": "zap-secret" }, + }, { actorType: "user", actorId: "board" })); expect(fetchMock).toHaveBeenCalledTimes(2); expect(fetchMock).toHaveBeenCalledWith( - "https://mcp.zapier.com/api/mcp", + PUBLIC_MCP_FIXTURE_URL, expect.objectContaining({ headers: expect.objectContaining({ Authorization: "Bearer zap-secret" }), }), @@ -5311,7 +5381,7 @@ describeEmbeddedPostgres("tool access service", () => { expect(connect.connection).toMatchObject({ status: "draft", enabled: false, - config: expect.objectContaining({ sourceTemplateKey: "zapier", quarantineNewEntries: true }), + config: expect.objectContaining({ sourceTemplateKey: "zapier", quarantineNewEntries: false }), credentialSecretRefs: [ expect.objectContaining({ configPath: "credentials.authorization", @@ -5391,6 +5461,10 @@ describeEmbeddedPostgres("tool access service", () => { ]), ); + await db.update(toolConnections).set({ + config: { ...connect.connection.config, quarantineNewEntries: true }, + }).where(eq(toolConnections.id, connect.connectionId)); + fetchMock.mockResolvedValueOnce(mcpHttpResponse({ jsonrpc: "2.0", id: "paperclip-catalog-refresh", @@ -5489,6 +5563,79 @@ describeEmbeddedPostgres("tool access service", () => { expect(attentionAfterReview.apps).toEqual([]); }); + it("enables every discovered tool by default while preserving tools explicitly turned off later", async () => { + const company = await createCompany(db); + const service = toolAccessService(db); + const fetchMock = mockToolsList([ + { name: "list_zaps", annotations: { readOnlyHint: true } }, + { name: "update_zap", annotations: { readOnlyHint: false } }, + ]); + + const connect = await withGalleryServerUrl("zapier", PUBLIC_MCP_FIXTURE_URL, () => + service.connectGalleryApp(company.id, { + galleryKey: "zapier", + credentialValues: { "credentials.authorization": "zap-secret" }, + }, { actorType: "user", actorId: "board" })); + const listEntry = connect.catalog.find((entry) => entry.toolName === "list_zaps")!; + const updateEntry = connect.catalog.find((entry) => entry.toolName === "update_zap")!; + const [defaultProfile] = await db.select().from(toolProfiles).where(eq( + toolProfiles.profileKey, + `app:${connect.connectionId}`, + )); + expect(defaultProfile).toBeTruthy(); + await expect(db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + defaultProfile!.id, + ))).resolves.toEqual(expect.arrayContaining([ + expect.objectContaining({ catalogEntryId: listEntry.id, effect: "include" }), + expect.objectContaining({ catalogEntryId: updateEntry.id, effect: "include" }), + ])); + await expect(db.select().from(toolProfileBindings).where(eq( + toolProfileBindings.profileId, + defaultProfile!.id, + ))).resolves.toEqual([ + expect.objectContaining({ targetType: "company", targetId: company.id }), + ]); + + await service.finishGalleryAppConnection(company.id, connect.connectionId, { + enabledCatalogEntryIds: [listEntry.id], + askFirstCatalogEntryIds: [], + access: "all_agents", + }, { actorType: "user", actorId: "board" }); + fetchMock.mockResolvedValueOnce(mcpHttpResponse({ + jsonrpc: "2.0", + id: "paperclip-catalog-refresh", + result: { + tools: [ + { name: "list_zaps", annotations: { readOnlyHint: true } }, + { name: "update_zap", annotations: { readOnlyHint: false } }, + { name: "create_zap", annotations: { readOnlyHint: false } }, + ], + }, + })); + + const refresh = await service.refreshCatalog(connect.connectionId, { actorType: "user", actorId: "board" }); + + expect(refresh.quarantinedCount).toBe(0); + expect(refresh.catalog).toEqual(expect.arrayContaining([ + expect.objectContaining({ toolName: "list_zaps", status: "active" }), + expect.objectContaining({ toolName: "update_zap", status: "active" }), + expect.objectContaining({ toolName: "create_zap", status: "active" }), + ])); + const createEntry = refresh.catalog.find((entry) => entry.toolName === "create_zap")!; + const profileEntries = await db.select().from(toolProfileEntries).where(eq( + toolProfileEntries.profileId, + defaultProfile!.id, + )); + expect(profileEntries).toEqual(expect.arrayContaining([ + expect.objectContaining({ catalogEntryId: listEntry.id }), + expect.objectContaining({ catalogEntryId: createEntry.id }), + ])); + expect(profileEntries).not.toEqual(expect.arrayContaining([ + expect.objectContaining({ catalogEntryId: updateEntry.id }), + ])); + }); + it("resolves Notion reads as allowed, mutations as ask-first, and denies cross-company use", async () => { const company = await createCompany(db); const otherCompany = await createCompany(db); diff --git a/server/src/routes/tool-access.ts b/server/src/routes/tool-access.ts index 42e8da566b2..287f7233c6e 100644 --- a/server/src/routes/tool-access.ts +++ b/server/src/routes/tool-access.ts @@ -57,6 +57,28 @@ function allowlistIds(config: Record | null | undefined): strin return raw.filter((value): value is string => typeof value === "string" && value.trim().length > 0); } +function agentOrgDepths(rows: Array<{ id: string; reportsTo: string | null }>): Map { + const parentById = new Map(rows.map((row) => [row.id, row.reportsTo])); + const depthById = new Map(); + + const depthFor = (agentId: string, path: Set): number => { + const known = depthById.get(agentId); + if (known !== undefined) return known; + const parentId = parentById.get(agentId); + if (!parentId || !parentById.has(parentId) || path.has(agentId)) { + depthById.set(agentId, 0); + return 0; + } + const nextPath = new Set(path).add(agentId); + const depth = depthFor(parentId, nextPath) + 1; + depthById.set(agentId, depth); + return depth; + }; + + for (const row of rows) depthFor(row.id, new Set()); + return depthById; +} + /** * Classify a connection PATCH into operator-visible lifecycle events so the * per-app Activity tab can humanize them (PAP-11284). A single update may @@ -740,9 +762,11 @@ export function toolAccessRoutes( role: agents.role, title: agents.title, status: agents.status, + reportsTo: agents.reportsTo, }) .from(agents) .where(eq(agents.companyId, connection.companyId)); + const orgDepthByAgentId = agentOrgDepths(rows); const candidates = []; for (const agent of rows) { try { @@ -751,7 +775,12 @@ export function toolAccessRoutes( continue; } candidates.push({ - ...agent, + id: agent.id, + name: agent.name, + role: agent.role, + title: agent.title, + status: agent.status, + orgDepth: orgDepthByAgentId.get(agent.id) ?? 0, effectiveAccess: await options.toolGateway.summarizeConnectionAccessForAgent({ companyId: connection.companyId, connectionId: connection.id, @@ -759,6 +788,7 @@ export function toolAccessRoutes( }), }); } + candidates.sort((a, b) => a.orgDepth - b.orgDepth || a.name.localeCompare(b.name)); res.json({ agents: candidates }); }); diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index dd5df47d9a1..422529d9258 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -3088,6 +3088,75 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} return profile; } + async function enableCatalogEntriesByDefault(input: { + connection: typeof toolConnections.$inferSelect; + newCatalogEntryIds: string[]; + activeCatalogEntryIds: string[]; + actor?: ActorInfo; + }) { + const profileKey = `app:${input.connection.id}`; + let [profile] = await db + .select() + .from(toolProfiles) + .where(and( + eq(toolProfiles.companyId, input.connection.companyId), + eq(toolProfiles.profileKey, profileKey), + )) + .limit(1); + const createdProfile = !profile; + if (!profile) { + [profile] = await db.insert(toolProfiles).values({ + companyId: input.connection.companyId, + profileKey, + name: input.connection.name, + description: `Access profile for ${input.connection.name}.`, + status: "active", + defaultAction: "deny", + metadata: { source: "app_gallery_finish", connectionId: input.connection.id }, + }).returning(); + await db.insert(toolProfileBindings).values({ + companyId: input.connection.companyId, + profileId: profile.id, + targetType: "company", + targetId: input.connection.companyId, + priority: 100, + metadata: { source: "app_gallery_finish" }, + createdByAgentId: input.actor?.actorType === "agent" ? input.actor.actorId ?? null : null, + createdByUserId: input.actor?.actorType === "user" ? input.actor.actorId ?? null : null, + }); + } + + // A new connection starts with every discovered action enabled. Later + // refreshes extend that managed profile only for genuinely new actions, so + // an action the operator deliberately turned off remains off. + const candidateIds = [...new Set( + createdProfile ? input.activeCatalogEntryIds : input.newCatalogEntryIds, + )]; + if (candidateIds.length === 0) return; + const existingEntries = await db + .select({ catalogEntryId: toolProfileEntries.catalogEntryId }) + .from(toolProfileEntries) + .where(and( + eq(toolProfileEntries.companyId, input.connection.companyId), + eq(toolProfileEntries.profileId, profile.id), + inArray(toolProfileEntries.catalogEntryId, candidateIds), + )); + const configuredIds = new Set(existingEntries.flatMap((entry) => + entry.catalogEntryId ? [entry.catalogEntryId] : [], + )); + const entryIds = candidateIds.filter((id) => !configuredIds.has(id)); + if (entryIds.length === 0) return; + await db.insert(toolProfileEntries).values(entryIds.map((catalogEntryId) => ({ + companyId: input.connection.companyId, + profileId: profile.id, + selectorType: "catalog_entry" as const, + effect: "include" as const, + applicationId: input.connection.applicationId, + connectionId: input.connection.id, + catalogEntryId, + }))); + } + async function listConnectionInstalls(connectionId: string, companyId?: string): Promise { const connection = await getConnectionRow(connectionId, companyId); const rows = await db @@ -4110,7 +4179,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} ? "quarantined" : existing?.status === "disabled" ? "disabled" - : existing?.status === "quarantined" + : quarantineOnRefresh && existing?.status === "quarantined" ? "quarantined" : "active"; if (shouldQuarantine) quarantinedCount += 1; @@ -4131,8 +4200,12 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} versionHash: hash, schemaHash, lastSeenAt: now, - quarantinedAt: shouldQuarantine ? now : existing.quarantinedAt, - quarantineReason: shouldQuarantine ? "pending_review" : existing.quarantineReason, + quarantinedAt: status === "quarantined" + ? shouldQuarantine ? now : existing.quarantinedAt + : null, + quarantineReason: status === "quarantined" + ? shouldQuarantine ? "pending_review" : existing.quarantineReason + : null, updatedAt: now, }) .where(eq(toolCatalogEntries.id, existing.id)) @@ -4188,6 +4261,19 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} .where(eq(toolRuntimeSlots.connectionId, connection.id)); } + 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), + activeCatalogEntryIds: activeEntries.map((entry) => entry.id), + actor, + }); + await audit({ companyId: connection.companyId, connectionId: connection.id, @@ -6305,10 +6391,10 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} sourceTemplateKey: galleryEntry.slug, connectionMethodKey: method?.key, methodConfig: normalizedMethodConfig?.values ?? {}, - quarantineNewEntries: true, + quarantineNewEntries: false, ...(galleryEntry.slug === "posthog" ? { safeDefault: true } : {}), } - : { ...baseConfig, quarantineNewEntries: true, unverifiedServer: true }; + : { ...baseConfig, quarantineNewEntries: false, unverifiedServer: true }; // A pasted URL may arrive with a client the operator preregistered in the // provider's own console, because that authorization server supports neither // CIMD nor dynamic registration. Record the client id now; the secret becomes diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index b0a8fbdc874..d57cf81a3ff 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -6,6 +6,7 @@ import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { AppDetail } from "./AppDetail"; +import { APP_TABS } from "./app-tabs"; const getConnectionMock = vi.hoisted(() => vi.fn()); const getConnectionInstallsMock = vi.hoisted(() => vi.fn()); @@ -269,6 +270,17 @@ describe("AppDetail", () => { await flushReact(); } + it("places Test immediately below Setup", () => { + expect(APP_TABS.map((tab) => tab.key)).toEqual([ + "setup", + "test", + "review", + "permissions", + "activity", + "advanced", + ]); + }); + it("pauses the app by flipping the connection enabled flag", async () => { await renderAppDetail(); @@ -311,19 +323,41 @@ describe("AppDetail", () => { }); it.each([ - ["setup", "Agents can use this app"], - ["review", "Review 1 new action"], - ["permissions", "Action permissions"], - ["activity", "No activity yet."], - ["advanced", "Technical details"], - ])("renders the %s tab panel", async (tab, expectedText) => { + ["setup", "Agents can use this app", false], + ["review", "Review 1 new action", true], + ["permissions", "Action permissions", true], + ["activity", "No activity yet.", false], + ["advanced", "Technical details", false], + ])("renders the %s tab panel", async (tab, expectedText, showsActionCount) => { mockParams.tab = tab; await renderAppDetail(); expect(container.textContent).toContain("GitHub"); - expect(container.textContent).toContain("2 actions available"); + expect(container.textContent?.includes("2 actions available")).toBe(showsActionCount); expect(container.textContent).toContain(expectedText); + expect(container.querySelector("section.bg-card")).toBeNull(); + }); + + it("renders setup without waiting for tool discovery", async () => { + listCatalogMock.mockImplementation(() => new Promise(() => undefined)); + + await renderAppDetail(); + + expect(container.textContent).toContain("Agents can use this app"); + expect(container.textContent).not.toContain("Loading tools"); + expect(listCatalogMock).not.toHaveBeenCalled(); + }); + + it("shows an explicit lazy-loading state while a tool tab discovers actions", async () => { + mockParams.tab = "permissions"; + listCatalogMock.mockImplementation(() => new Promise(() => undefined)); + + await renderAppDetail(); + + expect(container.textContent).toContain("GitHub"); + expect(container.textContent).toContain("Loading tools…"); + expect(container.textContent).not.toContain("Action permissions"); }); it("hides secret URL parameters in advanced technical details", async () => { @@ -421,6 +455,7 @@ describe("AppDetail", () => { expect(container.textContent).toContain("Agents can use this app"); expect(container.textContent).not.toContain("Read repo"); expect(container.textContent).not.toContain("Action permissions"); + expect(container.querySelector("section.bg-card")).toBeNull(); }); it("shows the Smoke OAuth connection action for the installed HTTP fixture", async () => { @@ -551,6 +586,7 @@ describe("AppDetail", () => { const writeSelect = container.querySelector('select[aria-label="Write issue permission"]'); expect(readSelect?.value).toBe("allowed"); expect(writeSelect?.value).toBe("ask"); + expect(container.querySelector("section.bg-card")).toBeNull(); }); it("persists ask-first for read-only actions from the unified dropdown", async () => { diff --git a/ui/src/pages/apps/AppDetail.tsx b/ui/src/pages/apps/AppDetail.tsx index 2f2740eca93..143dd668a01 100644 --- a/ui/src/pages/apps/AppDetail.tsx +++ b/ui/src/pages/apps/AppDetail.tsx @@ -62,6 +62,7 @@ export function AppDetail() { const { setBreadcrumbs } = useBreadcrumbs(); const activeTab: AppTabKey | null = isAppTabKey(tab) ? tab : null; + const needsCatalog = activeTab === "review" || activeTab === "permissions" || activeTab === "test"; const connectionQuery = useQuery({ queryKey: queryKeys.tools.connection(connectionId), @@ -81,22 +82,22 @@ export function AppDetail() { const catalogQuery = useQuery({ queryKey: queryKeys.tools.catalog(connectionId), queryFn: () => toolsApi.listCatalog(connectionId), - enabled: !!connectionId && !!activeTab, + enabled: !!connectionId && needsCatalog, }); const profilesQuery = useQuery({ queryKey: queryKeys.tools.profiles(selectedCompanyId ?? "__none__"), queryFn: () => toolsApi.listProfiles(selectedCompanyId!), - enabled: !!selectedCompanyId && !!activeTab, + enabled: !!selectedCompanyId && (activeTab === "review" || activeTab === "permissions"), }); const policiesQuery = useQuery({ queryKey: queryKeys.tools.policies(selectedCompanyId ?? "__none__"), queryFn: () => toolsApi.listPolicies(selectedCompanyId!), - enabled: !!selectedCompanyId && !!activeTab, + enabled: !!selectedCompanyId && (activeTab === "review" || activeTab === "permissions"), }); const agentsQuery = useQuery({ queryKey: queryKeys.agents.list(selectedCompanyId ?? "__none__"), queryFn: () => agentsApi.list(selectedCompanyId!), - enabled: !!selectedCompanyId && !!activeTab, + enabled: !!selectedCompanyId && (activeTab === "permissions" || activeTab === "activity"), }); const activityQuery = useQuery({ queryKey: queryKeys.tools.connectionActivity(connectionId), @@ -359,7 +360,7 @@ export function AppDetail() { if (!selectedCompanyId) { return
Select a company to manage apps.
; } - if (connectionQuery.isLoading || catalogQuery.isLoading) { + if (connectionQuery.isLoading) { return (
@@ -385,7 +386,11 @@ export function AppDetail() { const active = catalog.filter((e) => e.status !== "quarantined" && e.status !== "removed"); const readOnly = active.filter((e) => e.isReadOnly); const canChange = active.filter((e) => !e.isReadOnly); - const actionCount = active.length; + const actionCount = catalogQuery.data ? active.length : null; + const reviewLoading = catalogQuery.isLoading || profilesQuery.isLoading || policiesQuery.isLoading; + const permissionsLoading = reviewLoading || installsQuery.isLoading || agentsQuery.isLoading; + const reviewFailed = catalogQuery.isError || profilesQuery.isError || policiesQuery.isError; + const permissionsFailed = reviewFailed || installsQuery.isError || agentsQuery.isError; return (
@@ -435,36 +440,58 @@ export function AppDetail() { /> )} {activeTab === "review" && ( - + reviewFailed + ? { + void catalogQuery.refetch(); + void profilesQuery.refetch(); + void policiesQuery.refetch(); + }} /> + : reviewLoading + ? + : )} {activeTab === "permissions" && ( - apply({ access: next })} - onSaveInstall={(next) => persistInstall.mutate(next)} - onRefreshActions={() => refreshTools.mutate()} - onSetActionPermission={(id, next) => apply(actionPermissionMutation(id, next, enabledIds, askFirstIds))} - onReviewQuarantined={reviewQuarantined} - /> + permissionsFailed + ? { + void catalogQuery.refetch(); + void profilesQuery.refetch(); + void policiesQuery.refetch(); + void installsQuery.refetch(); + void agentsQuery.refetch(); + }} /> + : permissionsLoading + ? + : apply({ access: next })} + onSaveInstall={(next) => persistInstall.mutate(next)} + onRefreshActions={() => refreshTools.mutate()} + onSetActionPermission={(id, next) => apply(actionPermissionMutation(id, next, enabledIds, askFirstIds))} + onReviewQuarantined={reviewQuarantined} + /> )} {activeTab === "test" && ( - + catalogQuery.isError + ? { void catalogQuery.refetch(); }} /> + : catalogQuery.isLoading + ? + : )} {activeTab === "activity" && ( : null}
- - {actionCount} {actionCount === 1 ? "action" : "actions"} available - + {actionCount !== null && ( + + {actionCount} {actionCount === 1 ? "action" : "actions"} available + + )}
@@ -583,6 +612,24 @@ function AppDetailHeader({ ); } +function ToolsLoading() { + return ( +
+ + Loading tools… +
+ ); +} + +function ToolsLoadError({ onRetry }: { onRetry: () => void }) { + return ( +
+

Couldn’t load tools for this app.

+ +
+ ); +} + function unverifiedRemoteHost(connection: ToolConnection): string | null { const sourceTemplateKey = connection.config?.sourceTemplateKey ?? connection.transportConfig.sourceTemplateKey; if ( diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index b50e779460a..75987fa12fc 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -726,7 +726,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await render(); - expect(container.textContent).toContain("Step 1 of 4"); + expect(container.textContent).toContain("Step 1 of 3"); expect(container.textContent).toContain("Connect Zapier"); expect(container.textContent).toContain("Add MCP URL"); expect(container.querySelector('img[src="https://example.com/zapier.png"]')).toBeTruthy(); @@ -747,15 +747,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(connectAppMock).toHaveBeenCalledTimes(1); expect(connectAppMock.mock.calls[0]?.[1]).toMatchObject({ link: zapierUrl, name: "Zapier" }); - expect(container.textContent).toContain("Step 2 of 4"); - expect(container.querySelector('img[src="https://example.com/zapier.png"]')).toBeTruthy(); - - await act(async () => { - buttonByText("Continue with 1 action on")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); - }); - await flushReact(); - - expect(container.textContent).toContain("Step 3 of 4"); + expect(container.textContent).toContain("Step 2 of 3"); expect(container.querySelector('img[src="https://example.com/zapier.png"]')).toBeTruthy(); await act(async () => { @@ -793,7 +785,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { }); await flushReact(); - expect(container.textContent).toContain("Step 4 of 4"); + expect(container.textContent).toContain("Step 3 of 3"); expect(container.textContent).toContain("Install Zapier tools?"); expect(container.textContent).toContain("Not yet"); @@ -982,7 +974,7 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { await flushReact(); expect(connectAppMock).toHaveBeenCalledTimes(1); - expect(mockNavigate).toHaveBeenCalledWith("/apps/connect?byo=1&appKey=zapier&stage=actions"); + expect(mockNavigate).toHaveBeenCalledWith("/apps/connect?byo=1&appKey=zapier&stage=access"); const [, input] = connectAppMock.mock.calls[0]; expect(input).toMatchObject({ galleryKey: "zapier", name: "Zapier" }); }); @@ -1156,7 +1148,7 @@ describe("AppsConnect — guided generic MCP flow (PAP-17087)", () => { .toBe("127.0.0.1:47399/mcp"); }); - it("keeps the endpoint host and Unverified server visible from setup through review", async () => { + it("keeps the endpoint host visible while skipping action review", async () => { await render(); await gotoLinkFrame(container, "https://mcp.example.test/mcp"); @@ -1208,13 +1200,11 @@ describe("AppsConnect — guided generic MCP flow (PAP-17087)", () => { }); await flushReact(); - // Now on the review step — the label has to survive the step change. - expect(container.textContent).toContain("List things"); + expect(container.textContent).toContain("Who can use mcp.example.test?"); expect(container.textContent).toContain("Unverified server"); expect(container.textContent).toContain("mcp.example.test"); - expect( - Array.from(container.querySelectorAll('[role="switch"]')).map((toggle) => toggle.getAttribute("aria-label")), - ).toEqual(["List things allowed", "Search things allowed", "qa_delete_widget allowed"]); + expect(container.textContent).not.toContain("List things"); + expect(container.querySelectorAll('[role="switch"]')).toHaveLength(0); }); it("offers a curated setup as a convenience without leaving the generic flow", async () => { diff --git a/ui/src/pages/apps/AppsConnect.tsx b/ui/src/pages/apps/AppsConnect.tsx index f2c90595d14..06269f904f2 100644 --- a/ui/src/pages/apps/AppsConnect.tsx +++ b/ui/src/pages/apps/AppsConnect.tsx @@ -22,7 +22,6 @@ import type { FieldDef, ToolApplication, ToolConnection, - ToolAppConnectionActionSummary, } from "@paperclipai/shared"; import { credentialConfigPath, getAppDefinitionForUrl, getAvailableConnectionMethod, getAvailableConnectionMethods } from "@paperclipai/shared"; import { useNavigate, useParams, useSearchParams } from "@/lib/router"; @@ -67,12 +66,11 @@ import { } from "./generic-mcp-connect"; import { autoExtendNotice, INSTALL_ALL_WARNING, installInfoNotice, installPayload } from "@/lib/tool-installs"; -type Step = "gallery" | "key" | "actions" | "who" | "install" | "success"; +type Step = "gallery" | "key" | "who" | "install" | "success"; export type OAuthConnectPhase = "entry" | "starting" | "redirecting" | "error"; const ROUTE_STAGE_BY_STEP: Partial> = { key: "setup", - actions: "actions", who: "access", install: "install", success: "complete", @@ -86,26 +84,19 @@ function appConnectHref(appKey: string, step: Step): string { type AppAccessSelection = "all_agents" | { agentIds: string[] }; type InstallMode = "none" | "specific" | "all"; -const STEP_LABELS = ["Pick app", "Add your key", "Choose actions", "Choose access", "Install tools"]; +const STEP_LABELS = ["Pick app", "Add your key", "Choose access", "Install tools"]; const STEP_INDEX: Record, number> = { gallery: 0, key: 1, - actions: 2, - who: 3, - install: 4, -}; -const ZAPIER_STEP_INDEX: Record, number> = { - key: 0, - actions: 1, who: 2, install: 3, }; -const ZAPIER_STEP_LABELS = ["Add MCP URL", "Choose actions", "Choose access", "Install tools"]; - -function askFirstLevelsFrom(result: ConnectToolAppResult): string[] { - const raw = (result.suggestedDefaults as { askFirstRiskLevels?: unknown })?.askFirstRiskLevels; - return Array.isArray(raw) ? raw.filter((x): x is string => typeof x === "string") : ["write", "destructive"]; -} +const ZAPIER_STEP_INDEX: Record, number> = { + key: 0, + who: 1, + install: 2, +}; +const ZAPIER_STEP_LABELS = ["Add MCP URL", "Choose access", "Install tools"]; function isGoogleSheetsEntry(entry: AppDefinition | null): boolean { return entry?.slug === "google-sheets"; @@ -439,11 +430,11 @@ export function AppsConnect() { setConnectResult(result); const defaults: Record = {}; for (const a of result.actions.readOnly) defaults[a.catalogEntryId] = true; - for (const a of result.actions.canMakeChanges) defaults[a.catalogEntryId] = false; + for (const a of result.actions.canMakeChanges) defaults[a.catalogEntryId] = true; setEnabled(defaults); setInstallMode("none"); setInstallAgentIds(new Set()); - setAppStep("actions"); + setAppStep("who"); }, onError: (error) => { const details = error instanceof ApiError && error.body && typeof error.body === "object" @@ -553,19 +544,14 @@ export function AppsConnect() { const finishMutation = useMutation({ mutationFn: async () => { - const askFirstLevels = connectResult ? askFirstLevelsFrom(connectResult) : []; - const changeActions = connectResult?.actions.canMakeChanges ?? []; const enabledIds = Object.entries(enabled) .filter(([, on]) => on) .map(([id]) => id); - const askFirstIds = changeActions - .filter((a) => enabled[a.catalogEntryId] && askFirstLevels.includes(a.riskLevel)) - .map((a) => a.catalogEntryId); const selection: AppAccessSelection = access === "all" ? "all_agents" : { agentIds: Array.from(agentIds) }; const result = await toolsApi.finishApp(selectedCompanyId!, connectResult!.connectionId, { enabledCatalogEntryIds: enabledIds, - askFirstCatalogEntryIds: askFirstIds, + askFirstCatalogEntryIds: [], access: selection, }); const installState = installMode === "all" @@ -688,9 +674,9 @@ export function AppsConnect() { const stepLabels = zapierSource ? ZAPIER_STEP_LABELS : entry && getAvailableConnectionMethods(entry).length > 1 - ? ["Pick app", "Choose connection", "Choose actions", "Choose access", "Install tools"] + ? ["Pick app", "Choose connection", "Choose access", "Install tools"] : isGoogleSheetsEntry(entry) - ? ["Pick app", "Share sheet", "Choose actions", "Choose access", "Install tools"] + ? ["Pick app", "Share sheet", "Choose access", "Install tools"] : STEP_LABELS; const stepIndex = zapierSource && step !== "gallery" && step !== "success" ? ZAPIER_STEP_INDEX[step] @@ -841,24 +827,6 @@ export function AppsConnect() { /> )} - {step === "actions" && connectResult && ( - setEnabled((prev) => ({ ...prev, [id]: on }))} - onBulk={(ids, on) => - setEnabled((prev) => { - const next = { ...prev }; - for (const id of ids) next[id] = on; - return next; - }) - } - onBack={() => setAppStep("key")} - onContinue={() => setAppStep("who")} - /> - )} - {step === "who" && connectResult && ( setAppStep("actions")} + onBack={() => setAppStep("key")} onContinue={() => setAppStep("install")} /> )} @@ -1015,7 +983,7 @@ export function OAuthConnectStateScreen({ subtitle="Secure MCP sign-in" step="key" activeIndex={0} - labels={["Connect", "Review actions", "Choose access", "Install tools"]} + labels={["Connect", "Choose access", "Install tools"]} appIdentity={entry ? { name: entry.name, logoUrl: entry.branding.logoUrl } : undefined} unverifiedHost={unverifiedHost} onCancel={onCancel} @@ -1255,7 +1223,7 @@ function GalleryStep({ {zapierSource ? "Paste the complete MCP URL Zapier gives you, including its token." : byo - ? "Paste your MCP server’s URL and we’ll walk you through permissions and review." + ? "Paste your MCP server’s URL and every discovered tool will be available immediately." : "Paste a setup link from an app that is not listed here."}

{!zapierSource && ( @@ -2100,150 +2068,6 @@ function MethodConfigField({ ); } -function ActionGroup({ - title, - hint, - actions, - enabled, - onToggle, - bulkLabel, - onBulk, - askFirstLevels, -}: { - title: string; - hint: string; - actions: ToolAppConnectionActionSummary[]; - enabled: Record; - onToggle: (id: string, on: boolean) => void; - bulkLabel: string; - onBulk: () => void; - askFirstLevels: string[]; -}) { - if (actions.length === 0) return null; - return ( -
-
-
- {title} - · {hint} -
- -
-
- {actions.map((action) => { - const on = enabled[action.catalogEntryId] ?? false; - const showAskFirst = on && askFirstLevels.includes(action.riskLevel); - return ( -
-
-
- {action.title ?? action.toolName} -
- {action.description && ( -
{action.description}
- )} -
- {showAskFirst && ( - - Ask first - - )} - onToggle(action.catalogEntryId, next)} - /> -
- ); - })} -
-
- ); -} - -function ActionsStep({ - appName, - result, - enabled, - onToggle, - onBulk, - onBack, - onContinue, -}: { - appName: string; - result: ConnectToolAppResult; - enabled: Record; - onToggle: (id: string, on: boolean) => void; - onBulk: (ids: string[], on: boolean) => void; - onBack: () => void; - onContinue: () => void; -}) { - const askFirstLevels = askFirstLevelsFrom(result); - const { readOnly, canMakeChanges } = result.actions; - const total = readOnly.length + canMakeChanges.length; - const enabledCount = Object.values(enabled).filter(Boolean).length; - - return ( -
-
- - - -
-
- Connected to {appName} — it offers {total} {total === 1 ? "action" : "actions"}. -
-
- Read-only actions are on. Anything that can change something starts off — turn on the ones you want. -
-
-
- - onBulk(readOnly.map((a) => a.catalogEntryId), false)} - askFirstLevels={askFirstLevels} - /> - - onBulk(canMakeChanges.map((a) => a.catalogEntryId), true)} - askFirstLevels={askFirstLevels} - /> - -
- -
- - If {appName} adds new actions later, they start off until you review them. - - -
-
-
- ); -} - function WhoStep({ appName, companyId, diff --git a/ui/src/pages/apps/ReviewQueueCard.tsx b/ui/src/pages/apps/ReviewQueueCard.tsx index 412b14fff25..241fc944057 100644 --- a/ui/src/pages/apps/ReviewQueueCard.tsx +++ b/ui/src/pages/apps/ReviewQueueCard.tsx @@ -26,10 +26,12 @@ export function ReviewQueueCard({ connectionId, emptyState = "hidden", heading = "Waiting for your OK", + plain = false, }: { connectionId?: string; emptyState?: "hidden" | "reassure"; heading?: string; + plain?: boolean; }) { const { selectedCompanyId } = useCompany(); @@ -51,7 +53,7 @@ export function ReviewQueueCard({ if (items.length === 0) { if (emptyState === "hidden") return null; return ( -
+
Nothing is waiting for your OK right now.
); @@ -68,14 +70,22 @@ export function ReviewQueueCard({
{items.map((item) => ( - + ))}
); } -function ReviewRow({ companyId, item }: { companyId: string; item: ToolActionRequestListItem }) { +function ReviewRow({ + companyId, + item, + plain, +}: { + companyId: string; + item: ToolActionRequestListItem; + plain: boolean; +}) { const queryClient = useQueryClient(); const { pushToast } = useToast(); const [resolving, setResolving] = useState(null); @@ -140,7 +150,7 @@ function ReviewRow({ companyId, item }: { companyId: string; item: ToolActionReq const preview = item.request.previewMarkdown?.trim(); return ( -
+
{actionLabel(item)} {item.applicationName && ( diff --git a/ui/src/pages/apps/app-detail/AdvancedPanel.tsx b/ui/src/pages/apps/app-detail/AdvancedPanel.tsx index d9935710142..85200a240b5 100644 --- a/ui/src/pages/apps/app-detail/AdvancedPanel.tsx +++ b/ui/src/pages/apps/app-detail/AdvancedPanel.tsx @@ -44,8 +44,8 @@ function KeySection({ }) { const [open, setOpen] = useState(false); return ( -
-
+
+
@@ -62,7 +62,7 @@ function KeySection({ )}
{open && ( -
+
+

Technical details

Address
@@ -278,11 +278,11 @@ export function DangerZone({ }) { const [confirming, setConfirming] = useState(false); return ( -
-
+
+
Danger zone
-
+

Remove this app

diff --git a/ui/src/pages/apps/app-detail/PermissionsPanel.tsx b/ui/src/pages/apps/app-detail/PermissionsPanel.tsx index ee97f4cce24..90839e280a4 100644 --- a/ui/src/pages/apps/app-detail/PermissionsPanel.tsx +++ b/ui/src/pages/apps/app-detail/PermissionsPanel.tsx @@ -110,8 +110,8 @@ function AccessSection({ const canSave = draft.mode === "all" || draft.agentIds.size > 0; return ( -

-
+
+

Who can use it

{summary}

@@ -124,7 +124,7 @@ function AccessSection({
{editing && ( -
+