History

fix: refresh MCP source tool manifests (#862)

Jad committed 5 months ago

Commit 5ae2ce81 parent bd61b6e2 files · +145 −88Browse files

Unified · Split

2 files
  1. apps/cloud/src/services/sources-api.node.test.ts +48 −0
  2. packages/plugins/mcp/src/sdk/plugin.ts +97 −88
apps/cloud/src/services/sources-api.node.test.ts+48−0
@@ -457,6 +457,54 @@ describe("sources api (HTTP)", () => { · 456 unmodified lines
457457 }),
458458 );
459459
460+ it.effect("generic source refresh updates MCP source tool rows", () =>
461+ Effect.gen(function* () {
462+ let toolName = "before_refresh";
463+ const server = yield* serveMcpServer(() =>
464+ makeGreetingMcpServer({
465+ name: "cloud-refresh-mcp",
466+ toolName,
467+ text: "refresh-ok",
468+ }),
469+ );
470+ const org = `org_${crypto.randomUUID()}`;
471+ const namespace = `mcp_${crypto.randomUUID().replace(/-/g, "_")}`;
472+ const scopeId = ScopeId.make(org);
473+
474+ const added = yield* asOrg(org, (client) =>
475+ client.mcp.addSource({
476+ params: { scopeId },
477+ payload: {
478+ transport: "remote",
479+ name: "Cloud Refresh MCP",
480+ endpoint: server.endpoint,
481+ remoteTransport: "streamable-http",
482+ namespace,
483+ },
484+ }),
485+ );
486+ expect(added).toEqual({ namespace, toolCount: 1 });
487+
488+ const beforeTools = yield* asOrg(org, (client) =>
489+ client.sources.tools({ params: { scopeId, sourceId: namespace } }),
490+ );
491+ expect(beforeTools.map((tool) => tool.id)).toContain(`${namespace}.before_refresh`);
492+ expect(beforeTools.map((tool) => tool.id)).not.toContain(`${namespace}.after_refresh`);
493+
494+ toolName = "after_refresh";
495+ const refreshResult = yield* asOrg(org, (client) =>
496+ client.sources.refresh({ params: { scopeId, sourceId: namespace } }),
497+ );
498+ expect(refreshResult.refreshed).toBe(true);
499+
500+ const afterTools = yield* asOrg(org, (client) =>
501+ client.sources.tools({ params: { scopeId, sourceId: namespace } }),
502+ );
503+ expect(afterTools.map((tool) => tool.id)).not.toContain(`${namespace}.before_refresh`);
504+ expect(afterTools.map((tool) => tool.id)).toContain(`${namespace}.after_refresh`);
505+ }),
506+ );
507+
460508 it.effect("sources.remove deletes the source and it drops off sources.list", () =>
461509 Effect.gen(function* () {
462510 const org = `org_${crypto.randomUUID()}`;
packages/plugins/mcp/src/sdk/plugin.ts+97−88
@@ -1260,6 +1260,100 @@ const toMcpConfigEntry = ( · 1259 unmodified lines
12601260 return entry;
12611261 };
12621262
1263+const refreshSourceInternal = (
1264+ ctx: PluginCtx<McpBindingStore>,
1265+ sourceId: string,
1266+ scope: string,
1267+ allowStdio: boolean,
1268+) =>
1269+ Effect.gen(function* () {
1270+ const sd = yield* ctx.storage.getSourceConfig(sourceId, scope).pipe(
1271+ Effect.withSpan("mcp.plugin.load_source_config", {
1272+ attributes: { "mcp.source.namespace": sourceId },
1273+ }),
1274+ );
1275+ if (!sd) {
1276+ return yield* new McpConnectionError({
1277+ transport: "remote",
1278+ message: `No stored config for MCP source "${sourceId}"`,
1279+ });
1280+ }
1281+
1282+ const ci = yield* resolveConnectorInput(sourceId, scope, sd, ctx, allowStdio).pipe(
1283+ Effect.catchTag("McpAuthRequiredError", ({ message }) =>
1284+ Effect.fail(new McpConnectionError({ transport: sd.transport, message })),
1285+ ),
1286+ Effect.withSpan("mcp.plugin.resolve_connector", {
1287+ attributes: {
1288+ "mcp.source.namespace": sourceId,
1289+ "mcp.source.transport": sd.transport,
1290+ },
1291+ }),
1292+ );
1293+ const manifest = yield* discoverTools(createMcpConnector(ci)).pipe(
1294+ Effect.mapError(
1295+ ({ message }) =>
1296+ new McpToolDiscoveryError({
1297+ stage: "list_tools",
1298+ message: `MCP refresh failed: ${message}`,
1299+ }),
1300+ ),
1301+ Effect.withSpan("mcp.plugin.discover_tools", {
1302+ attributes: { "mcp.source.namespace": sourceId },
1303+ }),
1304+ );
1305+
1306+ const existing = yield* ctx.storage.getSource(sourceId, scope);
1307+ const sourceName = manifest.server?.name ?? existing?.name ?? sourceId;
1308+
1309+ yield* ctx
1310+ .transaction(
1311+ Effect.gen(function* () {
1312+ yield* ctx.storage.removeBindingsByNamespace(sourceId, scope);
1313+ yield* ctx.core.sources.unregister({ id: sourceId, targetScope: scope });
1314+
1315+ yield* ctx.storage.putBindings(
1316+ sourceId,
1317+ scope,
1318+ manifest.tools.map((e) => ({
1319+ toolId: `${sourceId}.${e.toolId}`,
1320+ binding: toBinding(e),
1321+ })),
1322+ );
1323+ yield* ctx.core.sources.register({
1324+ id: sourceId,
1325+ scope,
1326+ kind: "mcp",
1327+ name: sourceName,
1328+ url: sd.transport === "remote" ? sd.endpoint : undefined,
1329+ canRemove: true,
1330+ canRefresh: true,
1331+ canEdit: sd.transport === "remote",
1332+ tools: manifest.tools.map((e) => ({
1333+ name: e.toolId,
1334+ description: e.description ?? `MCP tool: ${e.toolName}`,
1335+ inputSchema: e.inputSchema,
1336+ outputSchema: mcpCallToolResultOutputSchema(e.outputSchema),
1337+ })),
1338+ });
1339+ }),
1340+ )
1341+ .pipe(
1342+ Effect.withSpan("mcp.plugin.persist_source", {
1343+ attributes: {
1344+ "mcp.source.namespace": sourceId,
1345+ "mcp.source.tool_count": manifest.tools.length,
1346+ },
1347+ }),
1348+ );
1349+
1350+ return { toolCount: manifest.tools.length };
1351+ }).pipe(
1352+ Effect.withSpan("mcp.plugin.refresh_source", {
1353+ attributes: { "mcp.source.namespace": sourceId },
1354+ }),
1355+ );
1356+
12631357 export const mcpPlugin = definePlugin((options?: McpPluginOptions) => {
12641358 const allowStdio = options?.dangerouslyAllowStdioMCP ?? false;
12651359 // Per-plugin-instance runtime holder. Captured by closures in
@@ -1695,93 +1789,7 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { · 429 unmodified lines
16951789 );
16961790
16971791 const refreshSource = (namespace: string, scope: string) =>
1698− Effect.gen(function* () {
1699− const sd = yield* ctx.storage.getSourceConfig(namespace, scope).pipe(
1700− Effect.withSpan("mcp.plugin.load_source_config", {
1701− attributes: { "mcp.source.namespace": namespace },
1702− }),
1703− );
1704− if (!sd) {
1705− return yield* new McpConnectionError({
1706− transport: "remote",
1707− message: `No stored config for MCP source "${namespace}"`,
1708− });
1709− }
1710−
1711− const ci = yield* resolveConnectorInput(namespace, scope, sd, ctx, allowStdio).pipe(
1712− Effect.catchTag("McpAuthRequiredError", ({ message }) =>
1713− Effect.fail(new McpConnectionError({ transport: sd.transport, message })),
1714− ),
1715− Effect.withSpan("mcp.plugin.resolve_connector", {
1716− attributes: {
1717− "mcp.source.namespace": namespace,
1718− "mcp.source.transport": sd.transport,
1719− },
1720− }),
1721− );
1722− const manifest = yield* discoverTools(createMcpConnector(ci)).pipe(
1723− Effect.mapError(
1724− ({ message }) =>
1725− new McpToolDiscoveryError({
1726− stage: "list_tools",
1727− message: `MCP refresh failed: ${message}`,
1728− }),
1729− ),
1730− Effect.withSpan("mcp.plugin.discover_tools", {
1731− attributes: { "mcp.source.namespace": namespace },
1732− }),
1733− );
1734−
1735− const existing = yield* ctx.storage.getSource(namespace, scope);
1736− const sourceName = manifest.server?.name ?? existing?.name ?? namespace;
1737−
1738− yield* ctx
1739− .transaction(
1740− Effect.gen(function* () {
1741− yield* ctx.storage.removeBindingsByNamespace(namespace, scope);
1742− yield* ctx.core.sources.unregister({ id: namespace, targetScope: scope });
1743−
1744− yield* ctx.storage.putBindings(
1745− namespace,
1746− scope,
1747− manifest.tools.map((e) => ({
1748− toolId: `${namespace}.${e.toolId}`,
1749− binding: toBinding(e),
1750− })),
1751− );
1752− yield* ctx.core.sources.register({
1753− id: namespace,
1754− scope,
1755− kind: "mcp",
1756− name: sourceName,
1757− url: sd.transport === "remote" ? sd.endpoint : undefined,
1758− canRemove: true,
1759− canRefresh: true,
1760− canEdit: sd.transport === "remote",
1761− tools: manifest.tools.map((e) => ({
1762− name: e.toolId,
1763− description: e.description ?? `MCP tool: ${e.toolName}`,
1764− inputSchema: e.inputSchema,
1765− outputSchema: mcpCallToolResultOutputSchema(e.outputSchema),
1766− })),
1767− });
1768− }),
1769− )
1770− .pipe(
1771− Effect.withSpan("mcp.plugin.persist_source", {
1772− attributes: {
1773− "mcp.source.namespace": namespace,
1774− "mcp.source.tool_count": manifest.tools.length,
1775− },
1776− }),
1777− );
1778−
1779− return { toolCount: manifest.tools.length };
1780− }).pipe(
1781− Effect.withSpan("mcp.plugin.refresh_source", {
1782− attributes: { "mcp.source.namespace": namespace },
1783− }),
1784− );
1792+ refreshSourceInternal(ctx, namespace, scope, allowStdio);
17851793
17861794 const getSource = (namespace: string, scope: string) =>
17871795 ctx.storage.getSource(namespace, scope).pipe(
@@ -2286,7 +2294,8 @@ export const mcpPlugin = definePlugin((options?: McpPluginOptions) => { · 498 unmodified lines
22862294
22872295 usagesForConnection: () => Effect.succeed([]),
22882296
2289− refreshSource: () => Effect.void,
2297+ refreshSource: ({ ctx, sourceId, scope }) =>
2298+ refreshSourceInternal(ctx, sourceId, scope, allowStdio).pipe(Effect.asVoid),
22902299
22912300 // Connection refresh for oauth2-minted sources is owned by the
22922301 // canonical `"oauth2"` ConnectionProvider that core registers via