From 84c2b30358f4e54081f5c3138bd07e2b3cc6b09a Mon Sep 17 00:00:00 2001 From: Dotta Date: Thu, 20 Aug 2026 15:15:50 +0000 Subject: [PATCH] feat(apps): confirm connections and cache MCP actions Co-Authored-By: Paperclip --- .../src/__tests__/tool-access-service.test.ts | 55 +++++++++++++- server/src/routes/tool-access.ts | 14 ++-- server/src/services/tool-access.ts | 71 ++++++++++++++----- ui/src/pages/apps/AppDetail.test.tsx | 18 ++++- ui/src/pages/apps/AppDetail.tsx | 22 +++++- 5 files changed, 153 insertions(+), 27 deletions(-) diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 0e0fe8f0e1b..0575a89338a 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -2904,6 +2904,59 @@ describeEmbeddedPostgres("tool access service", () => { ]); }); + it("serves persisted MCP actions until the cache expires and then refreshes them", async () => { + const company = await createCompany(db); + let currentTime = new Date("2026-08-20T12:00:00.000Z"); + let tools = [ + { + name: "cached_read", + description: "Read the cached value.", + annotations: { readOnlyHint: true }, + }, + ]; + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async () => mcpHttpResponse({ + jsonrpc: "2.0", + id: "paperclip-catalog-refresh", + result: { tools }, + })); + const service = toolAccessService(db, { + now: () => currentTime, + catalogCacheTtlMs: 60_000, + }); + const connected = await service.connectGalleryApp(company.id, { + link: "https://cache.example.test/mcp", + name: "Cached actions", + }, { actorType: "user", actorId: "board" }); + const discoveryCallsAfterConnect = fetchMock.mock.calls.length; + + const cached = await service.listCatalog(connected.connectionId); + + expect(cached.map((entry) => entry.toolName)).toContain("cached_read"); + expect(fetchMock).toHaveBeenCalledTimes(discoveryCallsAfterConnect); + + tools = [ + ...tools, + { + name: "fresh_read", + description: "Read a newly discovered value.", + annotations: { readOnlyHint: true }, + }, + ]; + currentTime = new Date(currentTime.getTime() + 60_001); + + const refreshed = await service.listCatalog(connected.connectionId); + + expect(refreshed.map((entry) => entry.toolName)).toContain("fresh_read"); + expect(fetchMock).toHaveBeenCalledTimes(discoveryCallsAfterConnect + 1); + + currentTime = new Date(currentTime.getTime() + 60_001); + fetchMock.mockRejectedValueOnce(new Error("temporary MCP outage")); + + await expect(service.listCatalog(connected.connectionId)).resolves.toEqual( + expect.arrayContaining([expect.objectContaining({ toolName: "fresh_read" })]), + ); + }); + it("requires an explicit PostHog method and projects validated project filters", async () => { const company = await createCompany(db); const service = toolAccessService(db); @@ -3469,7 +3522,7 @@ describeEmbeddedPostgres("tool access service", () => { expect(redirectCallbackRes.status).toBe(303); expect(redirectCallbackRes.headers.location).toBe( - `/${company.issuePrefix}/apps/${redirectConnectRes.body.connectionId}/setup?oauth=connected`, + `/${company.issuePrefix}/apps/${redirectConnectRes.body.connectionId}/test?success=1`, ); expect(fetchMock).toHaveBeenCalledTimes(6); await expect(db.select().from(toolOauthStates)).resolves.toHaveLength(0); diff --git a/server/src/routes/tool-access.ts b/server/src/routes/tool-access.ts index e98e55019da..efd43a221b7 100644 --- a/server/src/routes/tool-access.ts +++ b/server/src/routes/tool-access.ts @@ -146,14 +146,18 @@ export function toolAccessRoutes( return new URL("/api/tools/oauth/callback", configured).toString(); } - async function oauthSetupPath(companyId: string, connectionId: string) { + async function oauthAppPath( + companyId: string, + connectionId: string, + tab: "setup" | "test", + ) { const [company] = await db .select({ issuePrefix: companies.issuePrefix }) .from(companies) .where(eq(companies.id, companyId)) .limit(1); if (!company) throw new Error("OAuth callback connection belongs to a missing company"); - return `/${company.issuePrefix}/apps/${connectionId}/setup`; + return `/${company.issuePrefix}/apps/${connectionId}/${tab}`; } const access = accessService(db); @@ -429,7 +433,7 @@ export function toolAccessRoutes( oauth: callbackErrorCode === "oauth_authorization_denied" ? "denied" : "failed", }); if (callbackErrorCode) params.set("code", callbackErrorCode); - const setupPath = await oauthSetupPath(pendingState.companyId, pendingState.connectionId); + const setupPath = await oauthAppPath(pendingState.companyId, pendingState.connectionId, "setup"); res.redirect(303, `${setupPath}?${params.toString()}`); return; } @@ -446,8 +450,8 @@ export function toolAccessRoutes( }, }); if (acceptsHtml) { - const setupPath = await oauthSetupPath(result.connection.companyId, result.connection.id); - res.redirect(303, `${setupPath}?oauth=connected`); + const testPath = await oauthAppPath(result.connection.companyId, result.connection.id, "test"); + res.redirect(303, `${testPath}?success=1`); return; } res.json(result); diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 7fb00f1f9b7..b6f81657054 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -449,7 +449,7 @@ function sameOAuthIssuer(a: string | null | undefined, b: string | null | undefi const oauthRegistrationFlights = new Map>(); -async function oauthSingleFlight( +async function singleFlight( flights: Map>, key: string, operation: () => Promise, @@ -470,6 +470,8 @@ type ToolAccessServiceOptions = { deploymentExposure?: DeploymentExposure; trustedLocalStdioRuntimeHost?: string | null; now?: () => Date; + /** How long persisted remote MCP action discovery remains fresh. */ + catalogCacheTtlMs?: number; /** Test seam for deciding whether an OAuth client metadata URL is publicly resolvable. */ oauthClientMetadataLookup?: RemoteHttpEndpointLookup; }; @@ -1832,9 +1834,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const policySvc = toolAccessPolicyService(db); const now = options.now ?? (() => new Date()); const runtimeSupervisor = createToolRuntimeSupervisor(db, options); - // This map only removes duplicate work inside one service instance. The - // database refresh lease below is the cross-process serialization boundary. + // These maps remove duplicate work inside one service instance. OAuth also + // uses the database refresh lease below as its cross-process boundary. const oauthRefreshFlights = new Map>(); + const catalogRefreshFlights = new Map>(); + const catalogCacheTtlMs = Math.max(0, options.catalogCacheTtlMs ?? 15 * 60 * 1000); function allowPrivateRemoteEndpoints() { return options.deploymentMode !== "authenticated" || options.deploymentExposure !== "public"; @@ -4135,7 +4139,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} refreshOptions: { enableAllByDefault?: boolean } = {}, ): Promise { const connection = await getConnectionRow(connectionId); - const now = new Date(); + const refreshedAt = now(); let descriptors: McpToolDescriptor[]; try { descriptors = await discoverTools(connection); @@ -4204,14 +4208,14 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} status, versionHash: hash, schemaHash, - lastSeenAt: now, + lastSeenAt: refreshedAt, quarantinedAt: status === "quarantined" - ? shouldQuarantine ? now : existing.quarantinedAt + ? shouldQuarantine ? refreshedAt : existing.quarantinedAt : null, quarantineReason: status === "quarantined" ? shouldQuarantine ? "pending_review" : existing.quarantineReason : null, - updatedAt: now, + updatedAt: refreshedAt, }) .where(eq(toolCatalogEntries.id, existing.id)) .returning(); @@ -4235,9 +4239,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} status, versionHash: hash, schemaHash, - firstSeenAt: now, - lastSeenAt: now, - quarantinedAt: shouldQuarantine ? now : null, + firstSeenAt: refreshedAt, + lastSeenAt: refreshedAt, + quarantinedAt: shouldQuarantine ? refreshedAt : null, quarantineReason: shouldQuarantine ? "pending_review" : null, }).returning(); updatedEntries.push(toCatalogEntry(created)); @@ -4257,11 +4261,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} transportConfig: normalizedTransportConfig, healthStatus: "ok", healthMessage: "Tool catalog refreshed.", - healthCheckedAt: now, - lastHealthAt: now, - lastCatalogRefreshAt: now, + healthCheckedAt: refreshedAt, + lastHealthAt: refreshedAt, + lastCatalogRefreshAt: refreshedAt, lastError: null, - updatedAt: now, + updatedAt: refreshedAt, }) .where(eq(toolConnections.id, connection.id)) .returning(); @@ -4270,7 +4274,12 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} await ensureRuntimeSlot(updatedConnection); await db .update(toolRuntimeSlots) - .set({ healthStatus: "ok", healthMessage: "Approved stdio template is ready.", lastHealthCheckAt: now, updatedAt: now }) + .set({ + healthStatus: "ok", + healthMessage: "Approved stdio template is ready.", + lastHealthCheckAt: refreshedAt, + updatedAt: refreshedAt, + }) .where(eq(toolRuntimeSlots.connectionId, connection.id)); } @@ -5890,7 +5899,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} } const key = `${input.connection.id}:${input.redirectUri}`; - return oauthSingleFlight(oauthRegistrationFlights, key, async () => { + return singleFlight(oauthRegistrationFlights, key, async () => { const latest = await getConnectionRow(input.connection.id, input.connection.companyId); const latestConfigured = configuredOAuthClientForConnection(latest, input.endpoints.provider); if (latestConfigured.clientId) { @@ -6361,7 +6370,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} if (typeof oauth.tokenUrl !== "string" || typeof oauth.provider !== "string") return connection; const expiresAtMs = oauthExpiresAtMs(connection); if (expiresAtMs && expiresAtMs > Date.now() + 60_000) return connection; - return oauthSingleFlight(oauthRefreshFlights, connection.id, async () => { + return singleFlight(oauthRefreshFlights, connection.id, async () => { const lease = await acquireOAuthRefreshLease(connection); if (!lease.leaseId) return lease.connection; try { @@ -8355,11 +8364,37 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} listCatalog: async (connectionId: string, companyId?: string): Promise => { const connection = await getConnectionRow(connectionId, companyId); - const rows = await db + let rows = await db .select() .from(toolCatalogEntries) .where(eq(toolCatalogEntries.connectionId, connection.id)) .orderBy(desc(toolCatalogEntries.updatedAt)); + const cacheExpired = connection.transport === "mcp_remote" + && connection.status !== "archived" + && ( + rows.length === 0 + || !connection.lastCatalogRefreshAt + || connection.lastCatalogRefreshAt.getTime() <= now().getTime() - catalogCacheTtlMs + ); + if (cacheExpired) { + try { + await singleFlight( + catalogRefreshFlights, + connection.id, + () => refreshCatalog(connection.id, { actorType: "system", actorId: "tool_catalog_cache" }), + ); + rows = await db + .select() + .from(toolCatalogEntries) + .where(eq(toolCatalogEntries.connectionId, connection.id)) + .orderBy(desc(toolCatalogEntries.updatedAt)); + } catch (error) { + // A stale catalog remains useful when the remote server is temporarily + // unavailable. Empty caches still fail so callers never mistake “no + // actions discovered” for a successful lookup. + if (rows.length === 0) throw error; + } + } return rows.map((row) => toCatalogEntryForConnection(row, connection)); }, diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index 37febf5fc00..25d617f192f 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -25,6 +25,7 @@ const listUserDirectoryMock = vi.hoisted(() => vi.fn()); const getSessionMock = vi.hoisted(() => vi.fn()); const mockNavigate = vi.hoisted(() => vi.fn()); const mockParams = vi.hoisted(() => ({ connectionId: "conn-1", tab: "setup" as string | undefined })); +const mockSearchParams = vi.hoisted(() => ({ value: new URLSearchParams() })); const navigateComponentMock = vi.hoisted(() => vi.fn()); const navigateTopLevelMock = vi.hoisted(() => vi.fn()); @@ -80,7 +81,7 @@ vi.mock("@/lib/browserNavigation", () => ({ vi.mock("@/lib/router", () => ({ useParams: () => mockParams, useNavigate: () => mockNavigate, - useSearchParams: () => [new URLSearchParams(), vi.fn()], + useSearchParams: () => [mockSearchParams.value, vi.fn()], Navigate: ({ to, replace }: { to: string; replace?: boolean }) => { navigateComponentMock({ to, replace }); return
; @@ -189,6 +190,7 @@ describe("AppDetail", () => { document.body.appendChild(container); mockParams.connectionId = "conn-1"; mockParams.tab = "setup"; + mockSearchParams.value = new URLSearchParams(); getConnectionMock.mockResolvedValue(connection()); getConnectionInstallsMock.mockResolvedValue({ connectionId: "conn-1", installs: [] }); listGalleryMock.mockResolvedValue({ @@ -398,6 +400,20 @@ describe("AppDetail", () => { expect(container.querySelector(".animate-spin")).toBeTruthy(); }); + it("confirms a successful connection on Test and clears the one-time URL flag", async () => { + mockParams.tab = "test"; + mockSearchParams.value = new URLSearchParams("success=1"); + + await renderAppDetail(); + + expect(pushToastMock).toHaveBeenCalledWith({ + title: "GitHub connected", + body: "The connection is ready. You can test an action below.", + tone: "success", + }); + expect(mockNavigate).toHaveBeenCalledWith("/apps/conn-1/test", { replace: true }); + }); + it("hides secret URL parameters in setup technical details", async () => { mockParams.tab = "setup"; getConnectionMock.mockResolvedValue( diff --git a/ui/src/pages/apps/AppDetail.tsx b/ui/src/pages/apps/AppDetail.tsx index a981d64bc5a..4f9742ce6c5 100644 --- a/ui/src/pages/apps/AppDetail.tsx +++ b/ui/src/pages/apps/AppDetail.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { Check, Loader2, Pencil } from "lucide-react"; import type { @@ -11,7 +11,7 @@ import { humanizeConnectionDisplayName, isToolConnectionAttentionHealth as isAttentionHealthStatus, } from "@paperclipai/shared"; -import { Navigate, useParams, useNavigate } from "@/lib/router"; +import { Navigate, useParams, useNavigate, useSearchParams } from "@/lib/router"; import { useCompany } from "@/context/CompanyContext"; import { useBreadcrumbs } from "@/context/BreadcrumbContext"; import { useToast } from "@/context/ToastContext"; @@ -62,6 +62,7 @@ export { DangerZone, connectionAddress, connectionTransportLabel }; export function AppDetail() { const { connectionId = "", tab } = useParams<{ connectionId: string; tab?: string }>(); const navigate = useNavigate(); + const [searchParams] = useSearchParams(); const queryClient = useQueryClient(); const { pushToast } = useToast(); const { selectedCompany, selectedCompanyId } = useCompany(); @@ -138,6 +139,23 @@ export function AppDetail() { const appName = connection ? connectionDisplayNameForOwner(connection, baseAppName, owner) : "App"; + const successNoticeShownFor = useRef(null); + + useEffect(() => { + if ( + activeTab !== "test" + || searchParams.get("success") !== "1" + || !connection + || successNoticeShownFor.current === connection.id + ) return; + successNoticeShownFor.current = connection.id; + pushToast({ + title: `${appName} connected`, + body: "The connection is ready. You can test an action below.", + tone: "success", + }); + navigate(appTabHref(connection.id, "test"), { replace: true }); + }, [activeTab, appName, connection, navigate, pushToast, searchParams]); useEffect(() => { if (!activeTab) return;