Skip to content

Commit d75f2f4

Browse files
joeyorlandobrojd
andauthored
fix: new MCP server installation and logs UX (#2549)
Co-authored-by: brojd <[email protected]>
1 parent 31e16d4 commit d75f2f4

File tree

18 files changed

+1007
-266
lines changed

18 files changed

+1007
-266
lines changed

.github/values-ci.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ archestra:
55
# Image will be set via --set during deployment
66
# image: archestra/platform:ci-test
77

8+
# Deploy multiple replicas for the e2e environment to test issues we may face
9+
# which could be caused by storing state in pod memory, which may only be discovered
10+
# when replicas is > 1
11+
replicaCount: 5
12+
813
# Environment variables for e2e testing
914
env:
1015
ARCHESTRA_ANALYTICS: disabled

platform/backend/src/mcp-server-runtime/manager.test.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as fs from "node:fs";
2+
import { PassThrough } from "node:stream";
23
import * as k8s from "@kubernetes/client-node";
34
import { vi } from "vitest";
45
import type * as originalConfigModule from "@/config";
@@ -74,7 +75,11 @@ vi.mock("@/models/mcp-server", () => ({
7475
}));
7576

7677
vi.mock("./k8s-deployment", () => ({
77-
default: vi.fn(),
78+
default: class MockK8sDeployment {
79+
static sanitizeLabelValue(value: string): string {
80+
return value;
81+
}
82+
},
7883
}));
7984

8085
describe("validateKubeconfig", () => {
@@ -428,4 +433,38 @@ describe("McpServerRuntimeManager", () => {
428433
mockMakeApiClient.mockRestore();
429434
});
430435
});
436+
437+
describe("streamMcpServerLogs", () => {
438+
beforeEach(() => {
439+
vi.clearAllMocks();
440+
vi.resetModules();
441+
});
442+
443+
test("writes a helpful message when runtime is not configured", async () => {
444+
const mockLoadFromDefault = vi
445+
.spyOn(k8s.KubeConfig.prototype, "loadFromDefault")
446+
.mockImplementation(() => {
447+
throw new Error("Config load failed");
448+
});
449+
450+
const { McpServerRuntimeManager } = await import("./manager");
451+
const manager = new McpServerRuntimeManager();
452+
453+
const stream = new PassThrough();
454+
let output = "";
455+
stream.on("data", (chunk) => {
456+
output += chunk.toString();
457+
});
458+
459+
await manager.streamMcpServerLogs("test-server-id", stream);
460+
461+
expect(output).toContain("Unable to stream logs");
462+
expect(output).toContain(
463+
"Kubernetes runtime is not configured on this instance.",
464+
);
465+
expect(output).toContain("mcp-server-id=test-server-id");
466+
467+
mockLoadFromDefault.mockRestore();
468+
});
469+
});
431470
});

platform/backend/src/mcp-server-runtime/manager.ts

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -561,7 +561,8 @@ export class McpServerRuntimeManager {
561561
// Try to get from memory first, or lazy-load from database
562562
const k8sDeployment = await this.getOrLoadDeployment(mcpServerId);
563563
if (!k8sDeployment) {
564-
throw new Error(`MCP server not found`);
564+
this.writeLogsUnavailableMessage(responseStream, mcpServerId);
565+
return;
565566
}
566567

567568
await k8sDeployment.streamLogs(responseStream, lines, abortSignal);
@@ -658,6 +659,30 @@ export class McpServerRuntimeManager {
658659
await Promise.allSettled(stopPromises);
659660
logger.info("MCP Server Runtime shutdown complete");
660661
}
662+
663+
private writeLogsUnavailableMessage(
664+
responseStream: NodeJS.WritableStream,
665+
mcpServerId: string,
666+
): void {
667+
if ("destroyed" in responseStream && responseStream.destroyed) {
668+
return;
669+
}
670+
671+
const reason = this.k8sApi
672+
? "Deployment not loaded in runtime."
673+
: "Kubernetes runtime is not configured on this instance.";
674+
const command = this.getMcpServerDescribeCommand(mcpServerId);
675+
const message = [
676+
"Unable to stream logs for this MCP server.",
677+
reason,
678+
"Try running:",
679+
command,
680+
"",
681+
].join("\n");
682+
683+
responseStream.write(message);
684+
responseStream.end();
685+
}
661686
}
662687

663688
export default new McpServerRuntimeManager();

platform/backend/src/routes/internal-mcp-catalog.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -453,12 +453,22 @@ const internalMcpCatalogRoutes: FastifyPluginAsyncZod = async (fastify) => {
453453
try {
454454
for (const server of installedServers) {
455455
try {
456+
await McpServerModel.update(server.id, {
457+
localInstallationStatus: "pending",
458+
localInstallationError: null,
459+
});
456460
await autoReinstallServer(server, catalogItem);
461+
await McpServerModel.update(server.id, {
462+
localInstallationStatus: "success",
463+
localInstallationError: null,
464+
});
457465
logger.info(
458466
{ serverId: server.id, serverName: server.name },
459467
"Auto-reinstalled MCP server successfully",
460468
);
461469
} catch (error) {
470+
const errorMessage =
471+
error instanceof Error ? error.message : "Unknown error";
462472
logger.error(
463473
{
464474
err: error,
@@ -470,6 +480,8 @@ const internalMcpCatalogRoutes: FastifyPluginAsyncZod = async (fastify) => {
470480
// Mark for manual reinstall on failure
471481
await McpServerModel.update(server.id, {
472482
reinstallRequired: true,
483+
localInstallationStatus: "error",
484+
localInstallationError: errorMessage,
473485
});
474486
}
475487
}

platform/backend/src/test/fixtures.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ async function makeMember(
363363
*/
364364
async function makeMcpServer(
365365
overrides: Partial<
366-
Pick<InsertMcpServer, "name" | "catalogId" | "ownerId">
366+
Pick<InsertMcpServer, "name" | "catalogId" | "ownerId" | "teamId">
367367
> = {},
368368
) {
369369
// Create a catalog if catalogId is not provided

platform/backend/src/websocket.auth.test.ts

Lines changed: 0 additions & 115 deletions
This file was deleted.

platform/backend/src/websocket.browser-stream.test.ts

Lines changed: 0 additions & 98 deletions
This file was deleted.

0 commit comments

Comments
 (0)