Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 78 additions & 4 deletions test/e2e/live/mcp-bridge-tool-discovery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

import { expect } from "vitest";

import type { ArtifactSink } from "../fixtures/artifacts.ts";
import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts";
import { assertExitZero } from "../fixtures/clients/command.ts";
import type { HostCliClient } from "../fixtures/clients/host.ts";
Expand Down Expand Up @@ -39,8 +40,26 @@ export function shouldRetryMcpDiscoveryAfterRestart(
}

type McpToolDiscoveryStatusJson = {
provider: { credentialResolution?: unknown };
toolDiscovery: {
provider: Record<string, unknown> & {
registryPresent: boolean;
gatewayPresent: boolean | null;
attached: boolean | null;
credentialReady: boolean | null;
credentialResolution?: unknown;
};
policy: Record<string, unknown> & {
registryPresent: boolean;
gatewayPresent: boolean | null;
};
adapter: Record<string, unknown> & {
registered: boolean | null;
detail?: unknown;
};
trustedPrivateTarget?: Record<string, unknown> & {
state: "match" | "drift" | "unresolved";
detail?: unknown;
};
toolDiscovery: Record<string, unknown> & {
ok: boolean;
count: number;
tools: string[];
Expand All @@ -49,6 +68,56 @@ type McpToolDiscoveryStatusJson = {
};
};

function buildMcpToolDiscoveryDiagnostics(
status: McpToolDiscoveryStatusJson,
requests: readonly FakeMcpRequest[],
expectedSecret: string,
): Record<string, unknown> {
return {
provider: {
registryPresent: status.provider.registryPresent,
gatewayPresent: status.provider.gatewayPresent,
attached: status.provider.attached,
credentialReady: status.provider.credentialReady,
credentialResolutionPresent: status.provider.credentialResolution !== undefined,
},
policy: {
registryPresent: status.policy.registryPresent,
gatewayPresent: status.policy.gatewayPresent,
},
adapter: {
registered: status.adapter.registered,
detailPresent: status.adapter.detail !== undefined,
},
trustedPrivateTarget: status.trustedPrivateTarget
? {
state: status.trustedPrivateTarget.state,
detailPresent: status.trustedPrivateTarget.detail !== undefined,
}
: null,
toolDiscovery: {
ok: status.toolDiscovery.ok,
count: status.toolDiscovery.count,
tools: [...status.toolDiscovery.tools],
truncated: status.toolDiscovery.truncated,
...(status.toolDiscovery.detail !== undefined ? { detail: status.toolDiscovery.detail } : {}),
},
requests: requests.map((request) => ({
httpMethod: request.method,
rpcMethod: request.rpcMethod ?? null,
responseStatus: request.responseStatus ?? null,
responseHasResult: request.responseHasResult ?? null,
sessionMetadataPresent: {
sessionId: Boolean(request.sessionId),
protocolVersion: Boolean(request.protocolVersion),
negotiatedSessionId: Boolean(request.negotiatedSessionId),
negotiatedProtocolVersion: Boolean(request.negotiatedProtocolVersion),
},
credentialRewriteMatched: request.auth === `Bearer ${expectedSecret}`,
})),
};
}

export async function assertAuthenticatedMcpRediscovery(
target: AuthenticatedMcpDiscoveryTarget | undefined,
requestOffset: number | undefined,
Expand Down Expand Up @@ -174,6 +243,7 @@ export async function assertAuthenticatedMcpToolDiscovery(
host: HostCliClient,
fakeMcp: FakeMcpHttpsServer,
options: {
artifacts: Pick<ArtifactSink, "writeJson">;
sandboxName: string;
artifactPrefix: string;
credentialKey?: string;
Expand Down Expand Up @@ -202,7 +272,6 @@ export async function assertAuthenticatedMcpToolDiscovery(
);
assertExitZero(status, `${options.artifactPrefix} mcp status --tools --json`);
statusJson = JSON.parse(status.stdout) as McpToolDiscoveryStatusJson;
expect(statusJson.provider.credentialResolution).toBeUndefined();
if (
!shouldRetryMcpToolDiscoveryTransportFailure(
statusJson.toolDiscovery,
Expand All @@ -218,14 +287,19 @@ export async function assertAuthenticatedMcpToolDiscovery(
await new Promise((resolve) => setTimeout(resolve, MCP_TOOL_DISCOVERY_RETRY_DELAY_MS));
}
if (!status || !statusJson) throw new Error("MCP tool discovery did not run");
const discoveryRequests = fakeMcp.requests.slice(requestOffset);
await options.artifacts.writeJson(
`${options.artifactPrefix}-mcp-tool-discovery-diagnostics.json`,
buildMcpToolDiscoveryDiagnostics(statusJson, discoveryRequests, options.hostSecret),
);
expect(statusJson.provider.credentialResolution).toBeUndefined();
expect(statusJson.toolDiscovery).toMatchObject({
ok: true,
count: 2,
tools: ["fake_echo", "fake_status"],
truncated: false,
});
expect(status.stdout).not.toContain(options.hostSecret);
const discoveryRequests = fakeMcp.requests.slice(requestOffset);
const discoveryProtocolRequests = discoveryRequests.filter(
(request) =>
(request.method === "POST" || request.method === "DELETE") && request.path === "/mcp",
Expand Down
3 changes: 3 additions & 0 deletions test/e2e/live/mcp-bridge-trusted-private.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import type { ArtifactSink } from "../fixtures/artifacts.ts";
import { buildAvailabilityProbeEnv } from "../fixtures/availability-env.ts";
import type { CleanupRegistry } from "../fixtures/cleanup.ts";
import { assertExitZero as expectExitZero } from "../fixtures/clients/command.ts";
Expand Down Expand Up @@ -36,6 +37,7 @@ export async function assertTrustedPrivateMcpRebindingDenied(
cleanup: CleanupRegistry,
options: {
adapter: McpDnsRebindingAdapter;
artifacts: Pick<ArtifactSink, "writeJson">;
artifactPrefix: string;
assertSecretAbsent: (
sandbox: SandboxClient,
Expand Down Expand Up @@ -164,6 +166,7 @@ export async function assertTrustedPrivateMcpRebindingDenied(
`${options.artifactPrefix}-dns-rebinding-secret-absent-from-sandbox`,
);
await assertAuthenticatedMcpToolDiscovery(host, rebindMcp, {
artifacts: options.artifacts,
sandboxName: options.sandboxName,
artifactPrefix: `${options.artifactPrefix}-trusted-private`,
credentialKey: REBIND_CREDENTIAL_KEY,
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/live/mcp-bridge.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -779,6 +779,7 @@ test("mcp-bridge", {
artifactPrefix: "openclaw",
});
await assertAuthenticatedMcpToolDiscovery(host, fakeMcp, {
artifacts,
sandboxName: OPENCLAW_SANDBOX_NAME,
artifactPrefix: "openclaw",
hostSecret: HOST_SECRET,
Expand Down Expand Up @@ -848,6 +849,7 @@ test("mcp-bridge", {

await assertTrustedPrivateMcpRebindingDenied(host, sandbox, cleanup, {
adapter: "mcporter",
artifacts,
artifactPrefix: "openclaw",
assertSecretAbsent: assertSecretAbsentFromSandbox,
cleanupBridge: cleanupMcpBridge,
Expand Down Expand Up @@ -1096,6 +1098,7 @@ mcpBridgeShardTest("hermes")(
},
});
await assertAuthenticatedMcpToolDiscovery(host, fakeMcp, {
artifacts,
sandboxName: HERMES_SANDBOX_NAME,
artifactPrefix: "hermes",
hostSecret: HOST_SECRET,
Expand Down Expand Up @@ -1134,6 +1137,7 @@ mcpBridgeShardTest("hermes")(
};
await assertTrustedPrivateMcpRebindingDenied(host, sandbox, cleanup, {
adapter: "hermes-config",
artifacts,
artifactPrefix: "hermes",
assertSecretAbsent: assertSecretAbsentFromSandbox,
cleanupBridge: cleanupMcpBridge,
Expand Down Expand Up @@ -1321,6 +1325,7 @@ mcpBridgeShardTest("deepagents")(
artifactPrefix: "deepagents",
});
await assertAuthenticatedMcpToolDiscovery(host, fakeMcp, {
artifacts,
sandboxName: DEEPAGENTS_SANDBOX_NAME,
artifactPrefix: "deepagents",
hostSecret: HOST_SECRET,
Expand All @@ -1336,6 +1341,7 @@ mcpBridgeShardTest("deepagents")(
await assertSecretAbsentFromSandbox(sandbox, DEEPAGENTS_SANDBOX_NAME, ["/sandbox/.deepagents"]);
await assertTrustedPrivateMcpRebindingDenied(host, sandbox, cleanup, {
adapter: "deepagents-config",
artifacts,
artifactPrefix: "deepagents",
assertSecretAbsent: assertSecretAbsentFromSandbox,
cleanupBridge: cleanupMcpBridge,
Expand Down
104 changes: 104 additions & 0 deletions test/e2e/support/mcp-bridge-tool-discovery.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";

import { afterEach, describe, expect, it, vi } from "vitest";
import { ArtifactSink } from "../fixtures/artifacts.ts";
import { shouldRetryMcpMutationAfterConcurrencyConflict } from "../live/mcp-bridge-cleanup.ts";
import {
type FakeMcpHttpsServer,
Expand All @@ -12,6 +17,7 @@ import {
} from "../live/mcp-bridge-servers.ts";
import {
assertAuthenticatedMcpDiscoveryWithOneRestart,
assertAuthenticatedMcpToolDiscovery,
hasSuccessfulAuthenticatedMcpDiscovery,
shouldRetryMcpDiscoveryAfterRestart,
shouldRetryMcpToolDiscoveryTransportFailure,
Expand All @@ -21,6 +27,7 @@ const EXPECTED_SECRET = "expected-secret";
const EXPECTED_RESULT_TOKEN = "expected-result";
const SESSION_ID = "fake-session-1";
const PROTOCOL_VERSION = "2025-03-26";
const STATUS_SECRET = "unregistered-sensitive-status-value";

function request(rpcMethod: string, overrides: Partial<FakeMcpRequest> = {}): FakeMcpRequest {
return {
Expand Down Expand Up @@ -68,10 +75,14 @@ const BRIDGE_TOOLS = ["tool_search", "tool_describe", "tool_call"].map((name) =>
}));

let compatibleMock: StartedHttpServer | undefined;
const artifactRoots: string[] = [];

afterEach(async () => {
await compatibleMock?.close();
compatibleMock = undefined;
await Promise.all(
artifactRoots.splice(0).map((root) => fs.rm(root, { recursive: true, force: true })),
);
});

async function startDeferredCompatibleMock(): Promise<StartedHttpServer> {
Expand Down Expand Up @@ -185,6 +196,99 @@ describe("authenticated MCP rediscovery evidence", () => {
});

describe("authenticated MCP tool discovery transport retry", () => {
it("writes redacted boundary diagnostics before a discovery failure (#8746)", async () => {
const statusJson = {
provider: {
registryPresent: true,
gatewayPresent: true,
attached: true,
credentialReady: true,
credentialResolution: { detail: STATUS_SECRET },
token: STATUS_SECRET,
},
policy: { registryPresent: true, gatewayPresent: true, token: STATUS_SECRET },
adapter: { registered: true, detail: STATUS_SECRET, sessionId: STATUS_SECRET },
trustedPrivateTarget: {
state: "match" as const,
host: STATUS_SECRET,
recordedPins: [STATUS_SECRET],
detail: STATUS_SECRET,
},
toolDiscovery: {
ok: false,
count: 0,
tools: [],
truncated: false,
detail: "MCP tool discovery request failed",
credential: STATUS_SECRET,
},
};
const fakeMcp = { requests: [] } as unknown as FakeMcpHttpsServer;
const host = {
nemoclaw: vi.fn(async () => {
fakeMcp.requests.push(successfulInitialize());
return { exitCode: 0, stdout: JSON.stringify(statusJson), stderr: "" };
}),
} as unknown as Parameters<typeof assertAuthenticatedMcpToolDiscovery>[0];
const artifactRoot = await fs.mkdtemp(path.join(os.tmpdir(), "nemoclaw-mcp-diagnostics-"));
artifactRoots.push(artifactRoot);
const artifacts = new ArtifactSink(artifactRoot);

await expect(
assertAuthenticatedMcpToolDiscovery(host, fakeMcp, {
artifacts,
sandboxName: "sandbox",
artifactPrefix: "openclaw-trusted-private",
hostSecret: EXPECTED_SECRET,
progress: { event: vi.fn() },
}),
).rejects.toThrow();

const artifactPath = path.join(
artifactRoot,
"openclaw-trusted-private-mcp-tool-discovery-diagnostics.json",
);
const diagnostics = await fs.readFile(artifactPath, "utf8");
expect(JSON.parse(diagnostics)).toEqual({
provider: {
registryPresent: true,
gatewayPresent: true,
attached: true,
credentialReady: true,
credentialResolutionPresent: true,
},
policy: { registryPresent: true, gatewayPresent: true },
adapter: { registered: true, detailPresent: true },
trustedPrivateTarget: { state: "match", detailPresent: true },
toolDiscovery: {
ok: false,
count: 0,
tools: [],
truncated: false,
detail: "MCP tool discovery request failed",
},
requests: [
{
httpMethod: "POST",
rpcMethod: "initialize",
responseStatus: 200,
responseHasResult: true,
sessionMetadataPresent: {
sessionId: false,
protocolVersion: false,
negotiatedSessionId: true,
negotiatedProtocolVersion: true,
},
credentialRewriteMatched: true,
},
],
});
expect(diagnostics).not.toContain(STATUS_SECRET);
expect(diagnostics).not.toContain(EXPECTED_SECRET);
expect(diagnostics).not.toContain(SESSION_ID);
expect(diagnostics).not.toContain(PROTOCOL_VERSION);
});

it("retries one generic transport failure before any request reaches the fixture", () => {
expect(
shouldRetryMcpToolDiscoveryTransportFailure(
Expand Down
Loading