mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 10:26:26 +01:00
feat(apps): confirm connections and cache MCP actions
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
e6a7728087
commit
84c2b30358
5 files changed
+153
-27
No files matched your search
@@ -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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -449,7 +449,7 @@ function sameOAuthIssuer(a: string | null | undefined, b: string | null | undefi
|
||||
|
||||
const oauthRegistrationFlights = new Map<string, Promise<unknown>>();
|
||||
|
||||
async function oauthSingleFlight<T>(
|
||||
async function singleFlight<T>(
|
||||
flights: Map<string, Promise<unknown>>,
|
||||
key: string,
|
||||
operation: () => Promise<T>,
|
||||
@@ -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<string, Promise<unknown>>();
|
||||
const catalogRefreshFlights = new Map<string, Promise<unknown>>();
|
||||
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<ToolCatalogRefreshResult> {
|
||||
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<ToolCatalogEntry[]> => {
|
||||
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));
|
||||
},
|
||||
|
||||
|
||||
@@ -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 <div data-navigate-to={to} />;
|
||||
@@ -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(
|
||||
|
||||
@@ -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<string | null>(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;
|
||||
|
||||
Reference in new issue
Block a user