diff --git a/apps/web/src/mcp-servers.test.ts b/apps/web/src/mcp-servers.test.ts new file mode 100644 index 000000000..fa3ba3d84 --- /dev/null +++ b/apps/web/src/mcp-servers.test.ts @@ -0,0 +1,342 @@ +// The workspace MCP catalog: the workspace id resolves up from any +// workbench, and the built-in Exa row is stored once on the workspace — +// never on the workbench that triggered the ensure. + +import { MCP_STREAMABLE_HTTP_PROVIDER_KEY } from "@corbits/credential-mcp"; +import { EXA_MCP_SERVER, mcpCredentialName, mcpProviderName } from "@corbits/myra/workflow-ids"; +import { describe, expect, test } from "bun:test"; + +import { ensureBuiltInMcpServers, resolveWorkspaceTenantId } from "./mcp-servers"; + +const WORKSPACE_ID = "ws-1"; +const WORKBENCH_ID = "wb-1"; +const SIBLING_WORKBENCH_ID = "wb-2"; + +function pathOf(input: RequestInfo | URL): string { + if (typeof input === "string") return input; + if (input instanceof URL) return input.pathname; + return new URL(input.url).pathname; +} + +type Call = { readonly method: string; readonly path: string }; + +function tenantDetailResponse(id: string, parentId: string | null): Response { + return Response.json({ id, parentId }); +} + +describe("resolveWorkspaceTenantId", () => { + test("a top-level tenant resolves to itself", async () => { + const fetchImpl = (async () => + tenantDetailResponse(WORKSPACE_ID, null)) as unknown as typeof fetch; + await expect(resolveWorkspaceTenantId(WORKSPACE_ID, fetchImpl)).resolves.toBe(WORKSPACE_ID); + }); + + test("a workbench resolves up to its parent workspace", async () => { + const fetchImpl = (async () => + tenantDetailResponse(WORKBENCH_ID, WORKSPACE_ID)) as unknown as typeof fetch; + await expect(resolveWorkspaceTenantId(WORKBENCH_ID, fetchImpl)).resolves.toBe(WORKSPACE_ID); + }); + + test("a tenant detail without a parentId resolves to itself", async () => { + const fetchImpl = (async () => Response.json({ id: WORKSPACE_ID })) as unknown as typeof fetch; + await expect(resolveWorkspaceTenantId(WORKSPACE_ID, fetchImpl)).resolves.toBe(WORKSPACE_ID); + }); + + test("a failed tenant read throws instead of guessing", async () => { + const fetchImpl = (async () => + new Response("nope", { status: 404 })) as unknown as typeof fetch; + await expect(resolveWorkspaceTenantId(WORKBENCH_ID, fetchImpl)).rejects.toThrow(); + }); +}); + +type StoredRow = { + readonly id: string; + readonly name: string; + readonly providerId: string; + readonly metadata: { + readonly mcp: { + readonly handle: string; + readonly name: string; + readonly url: string; + readonly auth: string; + readonly tools: readonly never[]; + }; + }; +}; + +function exaRow(credentialId: string, providerId: string): StoredRow { + return { + id: credentialId, + name: mcpCredentialName(EXA_MCP_SERVER.handle), + providerId, + metadata: { + mcp: { + handle: EXA_MCP_SERVER.handle, + name: EXA_MCP_SERVER.name, + url: EXA_MCP_SERVER.url, + auth: "none", + tools: [], + }, + }, + }; +} + +function tokenRow(): StoredRow { + return { + id: "c-linear", + name: mcpCredentialName("linear"), + providerId: "p-linear-wb", + metadata: { + mcp: { + handle: "linear", + name: "Linear", + url: "https://mcp.linear.app/mcp", + auth: "token", + tools: [], + }, + }, + }; +} + +function workspaceCatalogFetch(opts: { + readonly exaPresent: boolean; + /** Legacy workbench-scoped rows, from before the catalog moved up. */ + readonly workbenchRows?: readonly StoredRow[]; +}): { + readonly fetchImpl: typeof fetch; + readonly calls: Call[]; +} { + const calls: Call[] = []; + const workspaceProviders = + opts.exaPresent === true + ? [ + { + id: "p-exa", + name: mcpProviderName(EXA_MCP_SERVER.handle), + plugin: MCP_STREAMABLE_HTTP_PROVIDER_KEY, + }, + ] + : []; + const workspaceCredentials = opts.exaPresent === true ? [exaRow("c-exa", "p-exa")] : []; + // Mutable: DELETEs drop rows, so tests can assert what moved and what stayed. + let workbenchCredentials = [...(opts.workbenchRows ?? [])]; + const fetchImpl = (async (input: RequestInfo | URL, init?: RequestInit) => { + const path = pathOf(input); + const method = init?.method ?? "GET"; + calls.push({ method, path }); + + if (path === `/api/tenants/${WORKBENCH_ID}`) + return tenantDetailResponse(WORKBENCH_ID, WORKSPACE_ID); + if (path === `/api/tenants/${WORKSPACE_ID}`) return tenantDetailResponse(WORKSPACE_ID, null); + if (path === `/api/tenants/${SIBLING_WORKBENCH_ID}`) + return tenantDetailResponse(SIBLING_WORKBENCH_ID, WORKSPACE_ID); + if ( + path === `/api/tenants/${SIBLING_WORKBENCH_ID}/providers` || + path === `/api/tenants/${SIBLING_WORKBENCH_ID}/credentials` + ) { + return Response.json({ data: [] }); + } + if (path === `/api/tenants/${WORKBENCH_ID}/providers` && method === "GET") { + return Response.json({ + data: workbenchCredentials.map((row) => ({ + id: row.providerId, + name: mcpProviderName(row.metadata.mcp.handle), + plugin: MCP_STREAMABLE_HTTP_PROVIDER_KEY, + })), + }); + } + if (path === `/api/tenants/${WORKBENCH_ID}/credentials` && method === "GET") { + return Response.json({ data: workbenchCredentials }); + } + if ( + method === "DELETE" && + (path.startsWith(`/api/tenants/${WORKBENCH_ID}/credentials/`) || + path.startsWith(`/api/tenants/${WORKBENCH_ID}/providers/`)) + ) { + const credentialId = path.split("/").pop() ?? ""; + workbenchCredentials = workbenchCredentials.filter( + (row) => row.id !== credentialId && row.providerId !== credentialId, + ); + return Response.json({ ok: true }); + } + if (path === `/api/tenants/${WORKSPACE_ID}/providers`) { + if (method === "GET") { + return Response.json({ data: workspaceProviders }); + } + const id = `p-added-${String(workspaceProviders.length)}`; + workspaceProviders.push({ + id, + name: mcpProviderName(EXA_MCP_SERVER.handle), + plugin: MCP_STREAMABLE_HTTP_PROVIDER_KEY, + }); + return Response.json({ + id, + name: mcpProviderName(EXA_MCP_SERVER.handle), + plugin: MCP_STREAMABLE_HTTP_PROVIDER_KEY, + }); + } + if (path === `/api/tenants/${WORKSPACE_ID}/credentials`) { + if (method === "GET") { + return Response.json({ data: workspaceCredentials }); + } + const bodyText = typeof init?.body === "string" ? init.body : "{}"; + let body: { metadata?: StoredRow["metadata"]; name?: string; providerId?: string } = {}; + try { + body = JSON.parse(bodyText) as typeof body; + } catch { + body = {}; + } + const row: StoredRow = { + id: `c-added-${String(workspaceCredentials.length)}`, + name: body.name ?? mcpCredentialName(EXA_MCP_SERVER.handle), + providerId: body.providerId ?? "p-exa", + metadata: body.metadata ?? exaRow("c-added-0", "p-exa").metadata, + }; + workspaceCredentials.push(row); + return Response.json({ id: row.id, name: row.name, providerId: row.providerId }); + } + if (path === `/api/tenants/${WORKSPACE_ID}/mcp/discover`) { + return Response.json({ data: { serverInfo: {}, tools: [] } }); + } + if (path.startsWith(`/api/tenants/${WORKSPACE_ID}/credentials/`)) { + const credentialId = path.split("/").pop() ?? ""; + const row = workspaceCredentials.find((candidate) => candidate.id === credentialId); + if (method === "PATCH" && row !== undefined) { + const patchText = typeof init?.body === "string" ? init.body : "{}"; + try { + const parsed = JSON.parse(patchText) as { metadata?: StoredRow["metadata"] }; + if (parsed.metadata !== undefined) { + workspaceCredentials.splice(workspaceCredentials.indexOf(row), 1, { + ...row, + metadata: parsed.metadata, + }); + } + } catch { + // Keep the placeholder row; discovery recorded nothing new. + } + } + return Response.json({ ok: true }); + } + throw new Error(`unexpected fetch: ${method} ${path}`); + }) as typeof fetch; + return { fetchImpl, calls }; +} + +describe("ensureBuiltInMcpServers", () => { + test("from a workbench id, an existing workspace Exa is shared with no writes", async () => { + const { fetchImpl, calls } = workspaceCatalogFetch({ exaPresent: true }); + const servers = await ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl); + expect(servers.map((server) => server.handle)).toContain(EXA_MCP_SERVER.handle); + // The migration check reads the workbench, but nothing is ever stored there. + const writes = calls.filter((call) => call.method !== "GET"); + expect(writes).toEqual([]); + }); + + test("a missing Exa is created on the workspace, not the workbench", async () => { + const { fetchImpl, calls } = workspaceCatalogFetch({ exaPresent: false }); + const servers = await ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl); + expect(servers.map((server) => server.handle)).toContain(EXA_MCP_SERVER.handle); + const writes = calls.filter((call) => call.method !== "GET"); + expect(writes.length).toBeGreaterThan(0); + for (const write of writes) { + expect(write.path.startsWith(`/api/tenants/${WORKSPACE_ID}/`)).toBe(true); + } + }); + + test("a legacy workbench Exa moves up to an empty workspace exactly once", async () => { + const { fetchImpl, calls } = workspaceCatalogFetch({ + exaPresent: false, + workbenchRows: [exaRow("c-exa-wb", "p-exa-wb")], + }); + const servers = await ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl); + expect(servers.map((server) => server.handle)).toContain(EXA_MCP_SERVER.handle); + // One re-add on the workspace (POST credentials), then the workbench row + // goes away — and no second add follows, because the moved row lists back. + const workspaceAdds = calls.filter( + (call) => call.method === "POST" && call.path === `/api/tenants/${WORKSPACE_ID}/credentials`, + ); + expect(workspaceAdds).toHaveLength(1); + expect( + calls.some( + (call) => + call.method === "DELETE" && + call.path === `/api/tenants/${WORKBENCH_ID}/credentials/c-exa-wb`, + ), + ).toBe(true); + }); + + test("a legacy workbench Exa dedupes against a workspace Exa instead of moving", async () => { + const { fetchImpl, calls } = workspaceCatalogFetch({ + exaPresent: true, + workbenchRows: [exaRow("c-exa-wb", "p-exa-wb")], + }); + const servers = await ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl); + expect(servers.map((server) => server.handle)).toContain(EXA_MCP_SERVER.handle); + // No re-add: the workspace already carries Exa, so the workbench twin is + // just dropped. + expect(calls.some((call) => call.method === "POST" && call.path.endsWith("/credentials"))).toBe( + false, + ); + expect( + calls.some( + (call) => + call.method === "DELETE" && + call.path === `/api/tenants/${WORKBENCH_ID}/credentials/c-exa-wb`, + ), + ).toBe(true); + }); + + test("a legacy token row stays orphaned on the workbench — its secret cannot move", async () => { + const { fetchImpl, calls } = workspaceCatalogFetch({ + exaPresent: true, + workbenchRows: [tokenRow()], + }); + await ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl); + expect( + calls.some((call) => call.method === "DELETE" && call.path.includes("/credentials/c-linear")), + ).toBe(false); + }); + + test("a failed migration still leaves the workspace catalog ensured", async () => { + const { fetchImpl } = workspaceCatalogFetch({ exaPresent: false }); + const failing: typeof fetch = (async (input: RequestInfo | URL, init?: RequestInit) => { + const path = pathOf(input); + if (path === `/api/tenants/${WORKBENCH_ID}/providers`) { + return new Response("gone", { status: 500 }); + } + return fetchImpl(input, init); + }) as typeof fetch; + const servers = await ensureBuiltInMcpServers(WORKBENCH_ID, failing); + expect(servers.map((server) => server.handle)).toContain(EXA_MCP_SERVER.handle); + }); + + test("concurrent ensures share one flight instead of adding Exa twice", async () => { + const { fetchImpl, calls } = workspaceCatalogFetch({ exaPresent: false }); + const [first, second] = await Promise.all([ + ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl), + ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl), + ]); + expect(first.map((server) => server.handle)).toContain(EXA_MCP_SERVER.handle); + expect(second.map((server) => server.handle)).toContain(EXA_MCP_SERVER.handle); + expect( + calls.filter( + (call) => + call.method === "POST" && call.path === `/api/tenants/${WORKSPACE_ID}/credentials`, + ), + ).toHaveLength(1); + }); + + test("sibling workbenches ensuring at once add the shared Exa once", async () => { + const { fetchImpl, calls } = workspaceCatalogFetch({ exaPresent: false }); + await Promise.all([ + ensureBuiltInMcpServers(WORKBENCH_ID, fetchImpl), + ensureBuiltInMcpServers(SIBLING_WORKBENCH_ID, fetchImpl), + ]); + expect( + calls.filter( + (call) => + call.method === "POST" && call.path === `/api/tenants/${WORKSPACE_ID}/credentials`, + ), + ).toHaveLength(1); + }); +}); diff --git a/apps/web/src/mcp-servers.ts b/apps/web/src/mcp-servers.ts index aa25d8a32..0c2e7923a 100644 --- a/apps/web/src/mcp-servers.ts +++ b/apps/web/src/mcp-servers.ts @@ -3,7 +3,12 @@ // provider row per server pins the credential handle to that server's origin; // the credential's `metadata.mcp` carries the catalog a deploy hands the // sidecar bundle, so redeploying never touches the network. +// +// The catalog lives once on the workspace (top-level) tenant: every +// workbench Myra binds it by walking up, so two workbenches share one Exa +// row and a second workspace sees none of it. import { MCP_NO_TOKEN_SENTINEL, MCP_STREAMABLE_HTTP_PROVIDER_KEY } from "@corbits/credential-mcp"; +import { reportError } from "@corbits/error-sink"; import { EXA_MCP_SERVER, MCP_SERVER_CATALOG, @@ -75,6 +80,32 @@ function tenantPath(tenantId: string, suffix: string): string { return `/api/tenants/${encodeURIComponent(tenantId)}${suffix}`; } +/** The only tenant-hierarchy read this module needs: a bench is a + * top-level tenant, so the workspace id is the tenant itself when it has no + * parent and its parent otherwise. Parsed rather than cast. */ +const TenantParentShape = type({ "parentId?": "string | null" }); + +/** Resolves the workspace (top-level) tenant for any tenant id: a workspace + * resolves to itself, a workbench to its parent. One hop only — workbenches + * are always direct children of the workspace that owns them, so a deeper + * chain would mean a hierarchy this catalog does not understand. */ +export async function resolveWorkspaceTenantId( + tenantId: string, + fetchImpl: typeof fetch = fetch, +): Promise { + const response = await fetchImpl(tenantPath(tenantId, ""), { + headers: { accept: "application/json" }, + }); + if (!response.ok) { + throw new McpServerError("resolving this workbench's workspace failed"); + } + const parsed = TenantParentShape(await response.json()); + if (parsed instanceof type.errors) { + throw new McpServerError("the workspace response came back an unexpected shape"); + } + return parsed.parentId ?? tenantId; +} + async function readJson( response: Response, shape: (value: unknown) => T | type.errors, @@ -93,9 +124,9 @@ async function listProviders( ): Promise { const listed = await fetchImpl(tenantPath(tenantId, "/providers")); if (!listed.ok) { - throw new McpServerError("listing this workbench's providers failed"); + throw new McpServerError("listing the MCP providers failed"); } - return (await readJson(listed, ProvidersPage, "this workbench's providers")).data; + return (await readJson(listed, ProvidersPage, "the MCP providers")).data; } async function listCredentials( @@ -104,13 +135,13 @@ async function listCredentials( ): Promise { const listed = await fetchImpl(tenantPath(tenantId, "/credentials")); if (!listed.ok) { - throw new McpServerError("listing this workbench's credentials failed"); + throw new McpServerError("listing the MCP credentials failed"); } - return (await readJson(listed, CredentialsPage, "this workbench's credentials")).data; + return (await readJson(listed, CredentialsPage, "the MCP credentials")).data; } -/** The workbench's MCP servers: the credentials sitting on an MCP provider, - * with the catalog each one recorded when it was added. */ +/** The MCP servers stored on a tenant: the credentials sitting on an MCP + * provider, with the catalog each one recorded when it was added. */ export async function listMcpServers( tenantId: string, fetchImpl: typeof fetch = fetch, @@ -243,7 +274,7 @@ export type AddMcpServerInput = { }; /** - * Adds a server to the workbench: provider, credential, then the catalog read + * Adds a server to the tenant: provider, credential, then the catalog read * with that credential. The credential is stored first because discovery of a * token-protected server needs the hub to hold the secret, and is dropped * again if the handshake fails, so a failed add leaves nothing behind. @@ -313,17 +344,96 @@ export async function removeMcpServer( }); } -/** Every workbench starts with Exa, which needs no account: added once, then - * left alone so a later removal is not undone by the next start. */ +/** Legacy workbench-scoped MCP rows predate the workspace catalog: before it, + * every workbench stored its own Exa. Moves the keyless (`auth === "none"`) + * rows up to the workspace — their sentinel secret is reconstructible, so + * re-adding rediscovers without touching a real credential — deduped by + * handle, then drops the workbench rows they came from. A token row carries + * a secret the client may never read, so it stays where it is: orphaned but + * untouched (accepted — the next redeploy binds the workspace catalog and a + * leftover row simply stops taking effect). */ +export async function migrateWorkbenchMcpCatalogToWorkspace( + workbenchTenantId: string, + workspaceTenantId: string, + fetchImpl: typeof fetch = fetch, +): Promise { + if (workbenchTenantId === workspaceTenantId) return; + const [workbenchServers, workspaceServers] = await Promise.all([ + listMcpServers(workbenchTenantId, fetchImpl), + listMcpServers(workspaceTenantId, fetchImpl), + ]); + const workspaceHandles = new Set(workspaceServers.map((server) => server.handle)); + for (const server of workbenchServers) { + // Only a keyless row can move without its secret; a token row stays + // orphaned on the workbench (see above). + if (server.auth !== "none") continue; + if (!workspaceHandles.has(server.handle)) { + const moved = await addMcpServer( + { + tenantId: workspaceTenantId, + handle: server.handle, + name: server.name, + url: server.url, + }, + fetchImpl, + ); + workspaceHandles.add(moved.handle); + } + await removeMcpServer( + { + tenantId: workbenchTenantId, + credentialId: server.credentialId, + providerId: server.providerId, + }, + fetchImpl, + ); + } +} + +/** Ensures that land on one workspace (two workbench Myras deploying, a + * StrictMode double-invoke) run one after another, so a later one sees the + * Exa an earlier one added instead of adding its own. */ +const ensureQueue = new Map>(); + +/** Every workspace starts with Exa, which needs no account: stored once on + * the workspace (top-level) tenant, then left alone so a later removal is + * not undone by the next start. Callers pass any tenant id — a workbench id + * resolves up to its workspace — and each workbench Myra binds the shared + * row by walking up. Also moves that caller's legacy workbench rows up, + * best-effort: a failed cleanup must not block deploying Myra with the + * workspace catalog that is already correct. */ export async function ensureBuiltInMcpServers( tenantId: string, fetchImpl: typeof fetch = fetch, ): Promise { - const existing = await listMcpServers(tenantId, fetchImpl); + const workspaceTenantId = await resolveWorkspaceTenantId(tenantId, fetchImpl); + const run = () => runEnsureBuiltInMcpServers(tenantId, workspaceTenantId, fetchImpl); + const previous = ensureQueue.get(workspaceTenantId); + // A failed earlier ensure already rejected to its own caller; this one runs regardless. + const flight = previous === undefined ? run() : previous.then(run, run); + ensureQueue.set(workspaceTenantId, flight); + const settle = () => { + if (ensureQueue.get(workspaceTenantId) === flight) ensureQueue.delete(workspaceTenantId); + }; + flight.then(settle, settle); + return flight; +} + +async function runEnsureBuiltInMcpServers( + tenantId: string, + workspaceTenantId: string, + fetchImpl: typeof fetch, +): Promise { + try { + await migrateWorkbenchMcpCatalogToWorkspace(tenantId, workspaceTenantId, fetchImpl); + } catch (cause) { + reportError(cause, { operation: "mcp_servers_catalog_migration" }); + } + const existing = await listMcpServers(workspaceTenantId, fetchImpl); if (existing.some((server) => server.handle === EXA_MCP_SERVER.handle)) return existing; const added = await addMcpServer( { - tenantId, + tenantId: workspaceTenantId, handle: EXA_MCP_SERVER.handle, name: EXA_MCP_SERVER.name, url: EXA_MCP_SERVER.url, diff --git a/apps/web/src/pages/tools-page.tsx b/apps/web/src/pages/tools-page.tsx index c68cbb71e..1e477315d 100644 --- a/apps/web/src/pages/tools-page.tsx +++ b/apps/web/src/pages/tools-page.tsx @@ -1,5 +1,5 @@ -// Two idioms, per DESIGN.md: a data table for what this workbench already -// carries, and a card catalog for the servers it could add. +// Two idioms, per DESIGN.md: a data table for what the workspace catalog +// already carries, and a card catalog for the servers it could add. import { useState } from "react"; import { @@ -27,6 +27,7 @@ import { Plugs } from "@/lib/icons"; import { describeApiError } from "@/lib/api-query"; import { MCP_SERVER_CATALOG, type McpCatalogEntry, type McpServer } from "../mcp-servers"; import { + describeRedeployResult, useAddMcpServer, useMcpServers, useRemoveMcpServer, @@ -230,9 +231,9 @@ export function ToolsPage({ tenantId }: { readonly tenantId: string | null }) { function addServer(input: { url: string; name: string; handle: string; token?: string }) { add.mutate(input, { - onSuccess: (server) => { + onSuccess: (result) => { toast( - `${server.name} added — Myra was redeployed with its ${String(server.tools.length)} tools.`, + `${result.server.name} added to the workspace catalog — ${describeRedeployResult(result)}`, ); }, onError: (cause: unknown) => { @@ -245,9 +246,9 @@ export function ToolsPage({ tenantId }: { readonly tenantId: string | null }) {
- + {(servers) => servers.length === 0 ? (

@@ -259,8 +260,10 @@ export function ToolsPage({ tenantId }: { readonly tenantId: string | null }) { removing={remove.isPending} onRemove={(server) => { remove.mutate(server, { - onSuccess: () => { - toast(`${server.name} removed — Myra was redeployed without it.`); + onSuccess: (result) => { + toast( + `${server.name} removed from the workspace catalog — ${describeRedeployResult(result)}`, + ); }, onError: (cause: unknown) => { toast(describeApiError(cause, "removing this server")); @@ -273,7 +276,7 @@ export function ToolsPage({ tenantId }: { readonly tenantId: string | null }) {

-
+
{MCP_SERVER_CATALOG.map((entry) => ( { + test("a failed redeploy is named, and the rest still go out", async () => { + const redeployed: string[] = []; + const result = await redeployMyraTenants( + [ + { id: "ws", name: "workspace" }, + { id: "wb-a", name: "Alpha" }, + { id: "wb-b", name: "Beta" }, + ], + { + findMyra: async () => MYRA, + redeploy: async (tenantId) => { + if (tenantId === "wb-a") throw new Error("boom"); + redeployed.push(tenantId); + }, + }, + ); + expect(result.redeployed).toBe(2); + expect(redeployed).toEqual(["ws", "wb-b"]); + expect(result.failed).toHaveLength(1); + expect(result.failed[0]).toMatchObject({ tenantId: "wb-a", workbenchName: "Alpha" }); + expect(result.failed[0]?.error).toBe("Something went wrong redeploying Myra. Try again."); + }); + + test("a workbench that cannot be read is named instead of stopping the loop", async () => { + const redeployed: string[] = []; + const result = await redeployMyraTenants( + [ + { id: "wb-a", name: "Alpha" }, + { id: "wb-b", name: "Beta" }, + ], + { + findMyra: async (tenantId) => { + if (tenantId === "wb-a") throw new Error("gone"); + return MYRA; + }, + redeploy: async (tenantId) => { + redeployed.push(tenantId); + }, + }, + ); + expect(result.redeployed).toBe(1); + expect(redeployed).toEqual(["wb-b"]); + expect(result.failed).toHaveLength(1); + expect(result.failed[0]).toMatchObject({ tenantId: "wb-a", workbenchName: "Alpha" }); + }); + + test("a workbench without Myra is skipped, not failed", async () => { + const result = await redeployMyraTenants([{ id: "wb-a", name: "Alpha" }], { + findMyra: async () => undefined, + redeploy: async () => { + throw new Error("must not be called"); + }, + }); + expect(result).toEqual({ redeployed: 0, failed: [] }); + }); +}); + +describe("describeRedeployResult", () => { + test("a clean redeploy names the count", () => { + expect(describeRedeployResult({ redeployed: 2, failed: [] })).toBe( + "Myra redeployed in 2 workbenches.", + ); + expect(describeRedeployResult({ redeployed: 1, failed: [] })).toBe( + "Myra redeployed in 1 workbench.", + ); + }); + + test("a partial failure names the stale workbenches", () => { + expect( + describeRedeployResult({ + redeployed: 1, + failed: [{ tenantId: "wb-a", workbenchName: "Alpha", error: "Something went wrong." }], + }), + ).toBe( + "Myra redeployed in 1 workbench, but the redeploy failed in Alpha — " + + "those workbenches still run the old catalog. Try the change again.", + ); + }); +}); diff --git a/apps/web/src/tools/mcp-servers-query.ts b/apps/web/src/tools/mcp-servers-query.ts index c2d2e288c..585605e99 100644 --- a/apps/web/src/tools/mcp-servers-query.ts +++ b/apps/web/src/tools/mcp-servers-query.ts @@ -1,18 +1,22 @@ -// The Tools page's reads and writes over the MCP servers a workbench holds. -// Adding or removing one changes what Myra carries, so every write ends in a -// redeploy: her definition's bindings are where a server actually takes effect. +// The Tools page's reads and writes over the workspace MCP catalog. The +// catalog lives once on the workspace (top-level) tenant regardless of which +// workbench is selected; adding or removing a server changes what every +// workbench Myra carries, so every write ends in a redeploy of each Myra +// whose binding list changed, and the page says how many. import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { reportError } from "@corbits/error-sink"; -import { toAPIQuery, type APIQuery } from "@/lib/api-query"; +import { describeApiError, toAPIQuery, type APIQuery } from "@/lib/api-query"; import { isMyraAgent, listChatAgents } from "../chat/threads-api"; +import { listWorkbenchTenants } from "../chat/workbench-tenants"; import { readAgentMcpHandles } from "../agent-source-read"; import { addMcpServer, listMcpServers, removeMcpServer, + resolveWorkspaceTenantId, type AddMcpServerInput, type McpServer, } from "../mcp-servers"; @@ -53,7 +57,11 @@ export function useMcpServers(tenantId: string | null): APIQuery { const id = tenantId as string; - const [servers, carriers] = await Promise.all([listMcpServers(id), carriersByHandle(id)]); + const workspaceTenantId = await resolveWorkspaceTenantId(id); + const [servers, carriers] = await Promise.all([ + listMcpServers(workspaceTenantId), + carriersByHandle(id), + ]); return servers.map((server) => ({ ...server, agentNames: [...(carriers.get(server.handle) ?? [])].sort((a, b) => a.localeCompare(b)), @@ -63,12 +71,95 @@ export function useMcpServers(tenantId: string | null): APIQuery { - const myra = (await listChatAgents(tenantId)).find(isMyraAgent); - if (myra === undefined) return; - await redeployWorkbenchAgent(tenantId, myra); +/** Myra binds the whole workspace catalog, so any catalog change alters her + * binding list in every workbench. Redeploys the workspace's own Myra and + * each child workbench's, and returns how many were redeployed. */ +export type RedeployFailure = { + /** The tenant whose Myra did not come back. */ + readonly tenantId: string; + /** The workbench name the Tools page can show, so a stale Myra is named. */ + readonly workbenchName: string; + /** User-safe copy: status-derived, never a raw path or schema summary. */ + readonly error: string; +}; + +export type RedeployWorkspaceResult = { + readonly redeployed: number; + readonly failed: readonly RedeployFailure[]; +}; + +type MyraRef = { readonly id: string; readonly name: string; readonly assetName: string }; + +/** The per-workbench loop behind `redeployWorkspaceMyras`, factored for + * test: one workbench's failure never stops the rest — the catalog write + * already landed, so failures come back named instead of failing the whole + * mutation. */ +export async function redeployMyraTenants( + tenants: readonly { readonly id: string; readonly name: string }[], + deps: { + readonly findMyra: (tenantId: string) => Promise; + readonly redeploy: (tenantId: string, myra: MyraRef) => Promise; + }, +): Promise { + let redeployed = 0; + const failed: RedeployFailure[] = []; + for (const tenant of tenants) { + let myra: MyraRef | undefined; + try { + myra = await deps.findMyra(tenant.id); + } catch (cause) { + failed.push({ + tenantId: tenant.id, + workbenchName: tenant.name, + error: describeApiError(cause, "reaching this workbench"), + }); + continue; + } + if (myra === undefined) continue; + try { + await deps.redeploy(tenant.id, myra); + redeployed += 1; + } catch (cause) { + failed.push({ + tenantId: tenant.id, + workbenchName: tenant.name, + error: describeApiError(cause, "redeploying Myra"), + }); + } + } + return { redeployed, failed }; +} + +/** One line the Tools page can toast after a catalog write: the count, plus + * the names a failed redeploy left stale. */ +export function describeRedeployResult(result: RedeployWorkspaceResult): string { + const count = `${String(result.redeployed)} ${result.redeployed === 1 ? "workbench" : "workbenches"}`; + if (result.failed.length === 0) return `Myra redeployed in ${count}.`; + const names = result.failed.map((failure) => failure.workbenchName).join(", "); + return ( + `Myra redeployed in ${count}, but the redeploy failed in ${names} — ` + + `those workbenches still run the old catalog. Try the change again.` + ); +} + +async function redeployWorkspaceMyras(workspaceTenantId: string): Promise { + const workbenches = await listWorkbenchTenants(workspaceTenantId); + const result = await redeployMyraTenants( + [ + { id: workspaceTenantId, name: "workspace" }, + ...workbenches.map((workbench) => ({ id: workbench.id, name: workbench.title })), + ], + { + findMyra: async (tenantId) => (await listChatAgents(tenantId)).find(isMyraAgent), + redeploy: (tenantId, myra) => redeployWorkbenchAgent(tenantId, myra), + }, + ); + for (const failure of result.failed) { + reportError(new Error(`redeploying Myra in ${failure.tenantId} failed`), { + operation: "mcp_servers_redeploy", + }); + } + return result; } function useInvalidateTools(tenantId: string | null) { @@ -83,30 +174,49 @@ function useInvalidateTools(tenantId: string | null) { }; } +export type AddMcpServerResult = { + readonly server: McpServer; + /** How many workbench Myras were redeployed with the new catalog. */ + readonly redeployed: number; + /** The workbenches whose redeploy failed — the catalog write already + * landed, so these still run the old one. */ + readonly failed: readonly RedeployFailure[]; +}; + export function useAddMcpServer(tenantId: string | null) { const invalidate = useInvalidateTools(tenantId); return useMutation({ - mutationFn: async (input: Omit) => { + mutationFn: async (input: Omit): Promise => { const id = tenantId as string; - const server = await addMcpServer({ tenantId: id, ...input }); - await redeployMyra(id); - return server; + const workspaceTenantId = await resolveWorkspaceTenantId(id); + const server = await addMcpServer({ tenantId: workspaceTenantId, ...input }); + const redeployed = await redeployWorkspaceMyras(workspaceTenantId); + return { server, ...redeployed }; }, onSettled: invalidate, }); } +export type RemoveMcpServerResult = { + /** How many workbench Myras were redeployed without the server. */ + readonly redeployed: number; + /** The workbenches whose redeploy failed — the removal already landed, + * so these still run the old catalog. */ + readonly failed: readonly RedeployFailure[]; +}; + export function useRemoveMcpServer(tenantId: string | null) { const invalidate = useInvalidateTools(tenantId); return useMutation({ - mutationFn: async (server: McpServer) => { + mutationFn: async (server: McpServer): Promise => { const id = tenantId as string; + const workspaceTenantId = await resolveWorkspaceTenantId(id); await removeMcpServer({ - tenantId: id, + tenantId: workspaceTenantId, credentialId: server.credentialId, providerId: server.providerId, }); - await redeployMyra(id); + return redeployWorkspaceMyras(workspaceTenantId); }, onSettled: invalidate, });