diff --git a/.changeset/connector-remove-button.md b/.changeset/connector-remove-button.md new file mode 100644 index 000000000..5581dd6e9 --- /dev/null +++ b/.changeset/connector-remove-button.md @@ -0,0 +1,5 @@ +--- +"@truefoundry/trueforge-ui": patch +--- + +Add a "Remove" button to configured connectors in Settings → Connectors, wiring the previously-unimplemented `deleteConnector` now that `DELETE /api/v1/settings/mcp-servers/{name}` exists (#495). diff --git a/.changeset/delete-mcp-server.md b/.changeset/delete-mcp-server.md new file mode 100644 index 000000000..f444c4590 --- /dev/null +++ b/.changeset/delete-mcp-server.md @@ -0,0 +1,5 @@ +--- +"@truefoundry/trueforge": minor +--- + +Add `DELETE /api/v1/settings/mcp-servers/{name}` to permanently remove a configured MCP server (its OAuth tokens and pending authorizations cascade-delete). Idempotent if already gone. diff --git a/packages/trueforge-ui/src/containers/SettingsBuilder/ConnectorSettings.tsx b/packages/trueforge-ui/src/containers/SettingsBuilder/ConnectorSettings.tsx index 9bd2529c4..1debc5425 100644 --- a/packages/trueforge-ui/src/containers/SettingsBuilder/ConnectorSettings.tsx +++ b/packages/trueforge-ui/src/containers/SettingsBuilder/ConnectorSettings.tsx @@ -227,6 +227,14 @@ const ConnectorSettings = () => { }).catch(() => {}); }; + const handleRemoveConnector = (connector: ConnectorBase) => { + const deleteConnector = connectorCatalog.deleteConnector; + if (!deleteConnector) return; + void runMutation(async () => { + await deleteConnector({ id: connector.id }); + }).catch(() => {}); + }; + const handleConnectorRefreshed = (refreshedConnector: ConnectorBase) => { setSelectedConnector(current => (current?.id === refreshedConnector.id ? refreshedConnector : current)); setConnectors(current => { @@ -312,6 +320,22 @@ const ConnectorSettings = () => { Replace Key ) : null} + {connectorCatalog.deleteConnector ? ( + + ) : null} diff --git a/packages/trueforge-ui/src/plugins/trueforge-agent-server-adapter/catalogs/connectorCatalog.ts b/packages/trueforge-ui/src/plugins/trueforge-agent-server-adapter/catalogs/connectorCatalog.ts index ec887c1e6..8d33a0f9c 100644 --- a/packages/trueforge-ui/src/plugins/trueforge-agent-server-adapter/catalogs/connectorCatalog.ts +++ b/packages/trueforge-ui/src/plugins/trueforge-agent-server-adapter/catalogs/connectorCatalog.ts @@ -113,7 +113,7 @@ export function toHarnessManifest(req: { }; } -/** Settings connector port for `createTrueFoundryServer`. Delete omitted; disconnect unsupported. */ +/** Settings connector port for `createTrueFoundryServer`. */ export function createConnectorCatalog( client: TrueForge, ): ConnectorCatalogServer< @@ -228,5 +228,8 @@ export function createConnectorCatalog( const body = await client.mcpServers.deleteAuthorization(req.id); return toUiConnector(body.data); }, + deleteConnector: async req => { + await client.settings.mcpServers.delete(req.id); + }, }; } diff --git a/packages/trueforge-ui/test/containers/SettingsBuilder/ConnectorSettings.test.tsx b/packages/trueforge-ui/test/containers/SettingsBuilder/ConnectorSettings.test.tsx new file mode 100644 index 000000000..47c17ea29 --- /dev/null +++ b/packages/trueforge-ui/test/containers/SettingsBuilder/ConnectorSettings.test.tsx @@ -0,0 +1,73 @@ +// @vitest-environment jsdom +import { fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { describe, expect, it, vi } from 'vitest'; + +import ConnectorSettings from '@/containers/SettingsBuilder/ConnectorSettings.js'; +import { ServerProvider } from '@/server/ServerContext.js'; +import type { ConnectorBase } from '@/server/types.js'; +import { createMockAgentUIServer, createMockCatalog } from '../../server/mockServer.js'; + +const connectedLinear: ConnectorBase = { + id: 'linear', + name: 'linear', + description: 'Search, read, and create Linear issues.', + url: 'https://mcp.linear.app/mcp', + auth: { type: 'dcr' }, + requiresAuth: false, + authenticated: true, +}; + +function renderConnectorSettings({ deleteConnector }: { deleteConnector?: (req: { id: string }) => Promise }) { + const server = createMockAgentUIServer({ + catalog: createMockCatalog({ + connectorCatalog: { + getConnectorCatalog: async () => [], + listConnectors: async () => [connectedLinear], + getConnector: async () => connectedLinear, + getToolsByConnectorId: async () => [], + createConnector: async () => connectedLinear, + updateConnector: async () => connectedLinear, + authenticateConnector: async () => ({ authorization_endpoint: '' }), + disconnectConnector: async () => connectedLinear, + ...(deleteConnector ? { deleteConnector } : {}), + }, + }), + }); + + render( + + + , + ); +} + +describe('ConnectorSettings Remove button (fixes #494)', () => { + it('hides Remove when the host has not wired deleteConnector', async () => { + renderConnectorSettings({}); + await screen.findByText('linear'); + expect(screen.queryByRole('button', { name: 'Remove linear' })).not.toBeInTheDocument(); + }); + + it('shows Remove and calls deleteConnector when the host supports it', async () => { + const deleteConnector = vi.fn(async () => undefined); + renderConnectorSettings({ deleteConnector }); + + const row = await screen.findByText('linear'); + const removeButton = within(row.closest('article') as HTMLElement).getByRole('button', { name: 'Remove linear' }); + fireEvent.click(removeButton); + + await waitFor(() => expect(deleteConnector).toHaveBeenCalledWith({ id: 'linear' })); + }); + + it('Remove does not open the connector details view (stops propagation)', async () => { + const deleteConnector = vi.fn(async () => undefined); + renderConnectorSettings({ deleteConnector }); + + const row = await screen.findByText('linear'); + const removeButton = within(row.closest('article') as HTMLElement).getByRole('button', { name: 'Remove linear' }); + fireEvent.click(removeButton); + + await waitFor(() => expect(deleteConnector).toHaveBeenCalled()); + expect(screen.queryByRole('button', { name: 'Connectors' })).not.toBeInTheDocument(); + }); +}); diff --git a/packages/trueforge/src/apis/mcpServers.ts b/packages/trueforge/src/apis/mcpServers.ts index 8a1c1d128..3b9d3d970 100644 --- a/packages/trueforge/src/apis/mcpServers.ts +++ b/packages/trueforge/src/apis/mcpServers.ts @@ -13,6 +13,7 @@ import { authorizeMcpServerRoute, createMcpServerRoute, deleteAuthorizationMcpServerRoute, + deleteMcpServerRoute, getMcpServerRoute, listAvailableMcpServersRoute, listMcpServersRoute, @@ -307,11 +308,18 @@ export function createSettingsMcpServersRouter(deps: McpServersRou } }; + const deleteHandler: RouteHandler = async c => { + const { name } = c.req.valid('param'); + await deps.mcpServerStore.deleteServer({ tenant_id: TENANT_ID, name }); + return c.json({}, 200); + }; + const router = new OpenAPIHono(); router.openapi(listMcpServersRoute, listHandler); router.openapi(createMcpServerRoute, createHandler); router.openapi(putMcpServerRoute, putHandler); router.openapi(getMcpServerRoute, getHandler); + router.openapi(deleteMcpServerRoute, deleteHandler); return router; } diff --git a/packages/trueforge/src/db/mcpServerStore.ts b/packages/trueforge/src/db/mcpServerStore.ts index 762f736c8..736edbee8 100644 --- a/packages/trueforge/src/db/mcpServerStore.ts +++ b/packages/trueforge/src/db/mcpServerStore.ts @@ -78,6 +78,11 @@ export interface IMcpServerStore extends IOAuthClientStore * Never overwrites `id`, `oauth_server`, or `oauth_client`. */ upsertServer(input: UpsertMcpServerInput, transaction?: TTransaction): Promise; + /** + * Permanently removes the server row. OAuth tokens and pending authorizations cascade-delete + * via their `oauth_server_id` foreign key. Idempotent if already gone. + */ + deleteServer(input: GetMcpServerInput, transaction?: TTransaction): Promise; } /** diff --git a/packages/trueforge/src/db/postgres/mcp-server-store/PostgresMcpServerStore.ts b/packages/trueforge/src/db/postgres/mcp-server-store/PostgresMcpServerStore.ts index 553c73ab2..264e7e16f 100644 --- a/packages/trueforge/src/db/postgres/mcp-server-store/PostgresMcpServerStore.ts +++ b/packages/trueforge/src/db/postgres/mcp-server-store/PostgresMcpServerStore.ts @@ -123,6 +123,11 @@ export class PostgresMcpServerStore implements IMcpServerStore): Promise { + const db = transaction ?? this.#db; + await db.deleteFrom('mcp_server').where('tenant_id', '=', input.tenant_id).where('name', '=', input.name).execute(); + } + async getClient(params: { id: string }, transaction?: Transaction): Promise { const db = transaction ?? this.#db; const row = await db diff --git a/packages/trueforge/src/db/sqlite/mcp-server-store/SqliteMcpServerStore.ts b/packages/trueforge/src/db/sqlite/mcp-server-store/SqliteMcpServerStore.ts index 2ef987ad8..11e9c628d 100644 --- a/packages/trueforge/src/db/sqlite/mcp-server-store/SqliteMcpServerStore.ts +++ b/packages/trueforge/src/db/sqlite/mcp-server-store/SqliteMcpServerStore.ts @@ -127,6 +127,11 @@ export class SqliteMcpServerStore implements IMcpServerStore): Promise { + const db = transaction ?? this.#db; + await db.deleteFrom('mcp_server').where('tenant_id', '=', input.tenant_id).where('name', '=', input.name).execute(); + } + async getClient(params: { id: string }, transaction?: Transaction): Promise { const db = transaction ?? this.#db; const row = await db diff --git a/packages/trueforge/src/routes/mcpServerRoutes.ts b/packages/trueforge/src/routes/mcpServerRoutes.ts index 9bfda2f62..02c4e3fdf 100644 --- a/packages/trueforge/src/routes/mcpServerRoutes.ts +++ b/packages/trueforge/src/routes/mcpServerRoutes.ts @@ -7,6 +7,7 @@ import { createRoute, z } from '@hono/zod-openapi'; import { RequestErrorResponseSchema } from '../schemas/errors'; import { CreateMcpServerRequestSchema, + DeleteMcpServerResponseSchema, GetMcpServerResponseSchema, ListAvailableMcpServersResponseSchema, ListMcpServersResponseSchema, @@ -157,6 +158,35 @@ export const putMcpServerRoute = createRoute({ }, }); +export const deleteMcpServerRoute = createRoute({ + method: 'delete', + path: '/{name}', + tags: [OpenApiTag.MCP_SERVERS], + summary: 'Delete an MCP server', + description: + 'Permanently removes the configured MCP server by name, including any stored OAuth tokens and ' + + 'pending authorizations. Idempotent if already gone.', + 'x-fern-sdk-group-name': ['settings', 'mcpServers'], + 'x-fern-sdk-method-name': 'delete', + request: { + params: McpServerNameParamsSchema, + }, + responses: { + 200: { + content: { 'application/json': { schema: DeleteMcpServerResponseSchema } }, + description: 'MCP server deleted.', + }, + 401: { + content: { 'application/json': { schema: RequestErrorResponseSchema } }, + description: 'OIDC is configured and the request has no valid session cookie.', + }, + 403: { + content: { 'application/json': { schema: RequestErrorResponseSchema } }, + description: 'OIDC is configured and the caller is authenticated but not an admin.', + }, + }, +}); + const ListMcpServerToolsResponseSchema = z .object({ // TODO: Type tools/list entries to the MCP tool shape (name, description, inputSchema, …) for OpenAPI quality. diff --git a/packages/trueforge/src/schemas/mcpServer.ts b/packages/trueforge/src/schemas/mcpServer.ts index ae4d3739c..19212d77d 100644 --- a/packages/trueforge/src/schemas/mcpServer.ts +++ b/packages/trueforge/src/schemas/mcpServer.ts @@ -102,6 +102,7 @@ export const GetMcpServerResponseSchema = z.object({ data: ConfiguredMcpServerSc export const ListMcpServersResponseSchema = z .object({ data: z.array(ConfiguredMcpServerSchema) }) .openapi('ListMCPServersResponse'); +export const DeleteMcpServerResponseSchema = z.object({}).openapi('DeleteMCPServerResponse'); /** Public auth mechanism for chat/composer (no secrets). */ export const McpServerAuthPublicSchema = z diff --git a/packages/trueforge/tests/db/mcpServerStoreContractSuite.ts b/packages/trueforge/tests/db/mcpServerStoreContractSuite.ts index 7d0d28f9f..ff3428d1f 100644 --- a/packages/trueforge/tests/db/mcpServerStoreContractSuite.ts +++ b/packages/trueforge/tests/db/mcpServerStoreContractSuite.ts @@ -137,6 +137,25 @@ export function runMcpServerStoreContractSuite(getStore: () => IMcpServerStore): await expect(store.listServers({ tenant_id: TENANT, names: [] })).resolves.toEqual([]); }); + it('deleteServer removes the row and cascade-clears its saved OAuth client', async () => { + const store = getStore(); + const created = await store.upsertServer({ tenant_id: TENANT, name: 'linear', manifest: manifest() }); + await store.saveClient({ id: created.id, record: sampleOAuthClient }); + + await store.deleteServer({ tenant_id: TENANT, name: 'linear' }); + + await expect(store.getServer({ tenant_id: TENANT, name: 'linear' })).resolves.toBeUndefined(); + await expect(store.getClient({ id: created.id })).resolves.toBeUndefined(); + }); + + it('deleteServer is idempotent for an unknown server and leaves other tenants untouched', async () => { + const store = getStore(); + const otherTenant = await store.upsertServer({ tenant_id: 'other-tenant', name: 'linear', manifest: manifest() }); + + await expect(store.deleteServer({ tenant_id: TENANT, name: 'linear' })).resolves.toBeUndefined(); + await expect(store.getServer({ tenant_id: 'other-tenant', name: 'linear' })).resolves.toEqual(otherTenant); + }); + it('upsert leaves oauth columns null and does not clear a saved OAuth client', async () => { const store = getStore(); const created = await store.upsertServer({ diff --git a/packages/trueforge/tests/unit/apis/mcpServers.test.ts b/packages/trueforge/tests/unit/apis/mcpServers.test.ts index 725c73239..1d217b2bf 100644 --- a/packages/trueforge/tests/unit/apis/mcpServers.test.ts +++ b/packages/trueforge/tests/unit/apis/mcpServers.test.ts @@ -870,4 +870,35 @@ describe('mcp-servers routers', () => { const missing = await mcpServersRouter.request('/missing/authorize', { method: 'DELETE' }); expect(missing.status).toBe(404); }); + + it('DELETE /{name} permanently removes the server and cascades its OAuth token', async () => { + const record = await seedDcrServerWithClient({ ...putBodyWithDcr, name: 'to-delete' }); + await tokenStore.saveToken({ + id: record.id, + userRef: LOCAL_USER_CONTEXT.userRef, + token: { + accessToken: 'access-1', + refreshToken: null, + expiresAt: '2099-01-01T00:00:00.000Z', + scope: null, + }, + }); + + const response = await settingsRouter.request('/to-delete', { method: 'DELETE' }); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({}); + + expect(await mcpServerStore.getServer({ tenant_id: TENANT_ID, name: 'to-delete' })).toBeUndefined(); + expect(await tokenStore.getToken({ id: record.id, userRef: LOCAL_USER_CONTEXT.userRef })).toBeUndefined(); + + const listed = await settingsRouter.request('/'); + const names = ((await listed.json()) as { data: { name: string }[] }).data.map(server => server.name); + expect(names).not.toContain('to-delete'); + }); + + it('DELETE /{name} is idempotent for an unknown server', async () => { + const response = await settingsRouter.request('/never-existed', { method: 'DELETE' }); + expect(response.status).toBe(200); + expect(await response.json()).toEqual({}); + }); });