Skip to content

Commit 6cc9779

Browse files
navidnadaliclaude
andcommitted
fix(server): MCP auth spans no longer include the tool call
The span on McpHttpServer.authenticateRequest wrapped the whole handled request, so traces attributed tool-call time to authentication. The auth span now covers only admitting the request, and each tool call gets its own mcp.tool.<name> span. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 8b44f42 commit 6cc9779

3 files changed

Lines changed: 110 additions & 8 deletions

File tree

‎apps/server/src/mcp/McpHttpServer.test.ts‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import * as Layer from "effect/Layer";
2424
import * as Logger from "effect/Logger";
2525
import * as Schema from "effect/Schema";
2626
import * as Stream from "effect/Stream";
27+
import * as Tracer from "effect/Tracer";
2728
import { McpSchema, McpServer, Tool, Toolkit } from "effect/ai";
2829
import { HttpBody, HttpClient, HttpClientRequest, HttpRouter } from "effect/http";
2930

@@ -140,6 +141,26 @@ const serveSnapshots = (clientId: string, result: unknown) =>
140141
return inputs;
141142
});
142143

144+
/** Records every span and the order spans start and end in, by span ID. */
145+
const recordSpans = () => {
146+
const spans: Array<Tracer.NativeSpan> = [];
147+
const order: Array<string> = [];
148+
const tracer = Tracer.make({
149+
span: (options) => {
150+
const span = new Tracer.NativeSpan(options);
151+
spans.push(span);
152+
order.push(`start ${span.spanId}`);
153+
const end = span.end.bind(span);
154+
span.end = (endTime, exit) => {
155+
end(endTime, exit);
156+
order.push(`end ${span.spanId}`);
157+
};
158+
return span;
159+
},
160+
});
161+
return { tracer, spans, order };
162+
};
163+
143164
const callTool = (name: string, args: Record<string, unknown>) =>
144165
Effect.gen(function* () {
145166
const server = yield* McpServer.McpServer;
@@ -178,19 +199,29 @@ it.effect.each([
178199
({ input, open, message, ...options }) =>
179200
Effect.gen(function* () {
180201
const server = yield* McpServer.McpServer;
202+
const traced = recordSpans();
181203
const result = yield* server.callTool({ name: "device_screenshot", arguments: input }).pipe(
182204
Effect.provideService(McpInvocationContext.McpInvocationContext, {
183205
...invocation,
184206
capabilities: new Set(["device"] as const),
185207
}),
186208
Effect.provideService(McpSchema.McpServerClient, client),
209+
Effect.withTracer(traced.tracer),
187210
);
188211
expect(result.isError).toBe(true);
189212
expect(result.content).toEqual([{ type: "text", text: message }]);
190213
expect(result.structuredContent).toMatchObject({
191214
error: { message, operation: "screenshot" },
192215
});
193216
expect(JSON.stringify(result)).not.toContain("private hub output");
217+
// The tool's span names it, never its arguments.
218+
expect(
219+
traced.spans
220+
.filter((span) => span.name.startsWith("mcp.tool."))
221+
.map((span) => Object.fromEntries(span.attributes)),
222+
).toEqual([
223+
{ "rpc.system": "mcp", "rpc.method": "tools/call", "mcp.tool.name": "device_screenshot" },
224+
]);
194225
}).pipe(
195226
Effect.provide(
196227
McpHttpServer.layerDeviceToolkit.pipe(
@@ -904,8 +935,10 @@ it.effect("admits provider and OAuth client credentials and points only clients
904935
}),
905936
),
906937
);
938+
const traced = recordSpans();
907939
yield* HttpRouter.serve(serverLayer, { disableListenLog: true, disableLogger: true }).pipe(
908940
Layer.build,
941+
Effect.withTracer(traced.tracer),
909942
);
910943
const httpClient = yield* HttpClient.HttpClient;
911944
const call = (token: string | undefined) =>
@@ -936,6 +969,21 @@ it.effect("admits provider and OAuth client credentials and points only clients
936969
});
937970

938971
expect((yield* call(providerToken)).status).toBe(200);
972+
// The tool gets its own span, without its arguments, which starts once authentication has ended.
973+
const toolSpan = traced.spans.find((span) => span.name === "mcp.tool.probe");
974+
expect(Object.fromEntries(toolSpan?.attributes ?? [])).toEqual({
975+
"rpc.system": "mcp",
976+
"rpc.method": "tools/call",
977+
"mcp.tool.name": "probe",
978+
});
979+
const authSpan = traced.spans.find(
980+
(span) =>
981+
span.name === "McpHttpServer.authenticateRequest" && span.traceId === toolSpan?.traceId,
982+
);
983+
expect(authSpan).toBeDefined();
984+
expect(traced.order.slice(0, traced.order.indexOf(`start ${toolSpan?.spanId}`))).toContain(
985+
`end ${authSpan?.spanId}`,
986+
);
939987
expect((yield* call(clientToken)).status).toBe(200);
940988
expect(seen.map((scope) => scope.requestNamespace)).toEqual([
941989
"provider-session",
@@ -952,9 +1000,22 @@ it.effect("admits provider and OAuth client credentials and points only clients
9521000
expect(expiredClient.headers["www-authenticate"]).toContain("resource_metadata=");
9531001
expect(expiredClient.headers["www-authenticate"]).toContain('error="invalid_token"');
9541002

1003+
const spansBeforeRejection = traced.spans.length;
9551004
const deadProvider = yield* call("deadProviderToken");
9561005
expect(deadProvider.status).toBe(401);
9571006
expect(deadProvider.headers["www-authenticate"]).toBe('Bearer error="invalid_token"');
1007+
// A rejected credential is recorded on the auth span, and nothing is handled.
1008+
const rejectedSpans = traced.spans.slice(spansBeforeRejection);
1009+
expect(
1010+
rejectedSpans
1011+
.filter((span) => span.name === "McpHttpServer.authenticateRequest")
1012+
.map((span) => span.events.map(([name]) => name)),
1013+
).toEqual([[expect.stringContaining("unknown_or_expired_token")]]);
1014+
expect(
1015+
rejectedSpans.filter(
1016+
(span) => span.name.startsWith("mcp.tool.") || span.name.startsWith("McpServer."),
1017+
),
1018+
).toEqual([]);
9581019
}),
9591020
).pipe(Effect.provide(NodeHttpServer.layerTest)),
9601021
);

‎apps/server/src/mcp/McpHttpServer.ts‎

Lines changed: 44 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,9 @@ const makeMcpAuthMiddleware = Effect.gen(function* () {
203203
Effect.map((settings) => settings.linkedEnvironmentsEnabled),
204204
Effect.orElseSucceed(() => false),
205205
);
206-
return Effect.fn("McpHttpServer.authenticateRequest")(function* (httpEffect) {
207-
const request = yield* HttpServerRequest.HttpServerRequest;
206+
const authenticateRequest = Effect.fn("McpHttpServer.authenticateRequest")(function* (
207+
request: HttpServerRequest.HttpServerRequest,
208+
) {
208209
const authorization = request.headers.authorization;
209210
const token = parseBearerToken(request) ?? "";
210211
const invocation =
@@ -234,11 +235,14 @@ const makeMcpAuthMiddleware = Effect.gen(function* () {
234235
const limited = McpInvocationContext.withRuntimeModeCeiling(asked, runtimeModeCeiling);
235236
const linkedEnvironments = yield* linksEnabled;
236237
if (limited.client?.linked === true && !linkedEnvironments) return linkedEnvironmentsOff;
238+
return { ...limited, linkedEnvironments };
239+
});
240+
// Untraced: a span here would wrap the tool call; the HTTP server span already covers it.
241+
return Effect.fnUntraced(function* (httpEffect) {
242+
const admitted = yield* authenticateRequest(yield* HttpServerRequest.HttpServerRequest);
243+
if (HttpServerResponse.isHttpServerResponse(admitted)) return admitted;
237244
return yield* httpEffect.pipe(
238-
Effect.provideService(McpInvocationContext.McpInvocationContext, {
239-
...limited,
240-
linkedEnvironments,
241-
}),
245+
Effect.provideService(McpInvocationContext.McpInvocationContext, admitted),
242246
Effect.map(normalizeMcpHttpResponse),
243247
);
244248
}) satisfies McpAuthMiddleware;
@@ -1022,6 +1026,36 @@ const withToolAnalytics =
10221026
return yield* registration.pipe(Effect.provideService(McpServer.McpServer, recordingServer));
10231027
});
10241028

1029+
/** Each tool call gets a span named for its tool; arguments stay out, as they carry prompts and IDs. */
1030+
const withToolSpans = Effect.fn("McpHttpServer.withToolSpans")(function* <A, E, R>(
1031+
registration: Effect.Effect<A, E, R>,
1032+
) {
1033+
const server = yield* McpServer.McpServer;
1034+
const tracingServer = McpServer.McpServer.of({
1035+
...server,
1036+
addTool: Effect.fn("McpServer.addTool")(function (options) {
1037+
const name = options.tool.name;
1038+
return server.addTool({
1039+
...options,
1040+
// Never the current span: Toolkit.handle annotates that one with the raw arguments.
1041+
handle: (payload) =>
1042+
Effect.useSpan(
1043+
`mcp.tool.${name}`,
1044+
{
1045+
attributes: {
1046+
"rpc.system": "mcp",
1047+
"rpc.method": "tools/call",
1048+
"mcp.tool.name": name,
1049+
},
1050+
},
1051+
() => options.handle(payload),
1052+
),
1053+
});
1054+
}),
1055+
});
1056+
return yield* registration.pipe(Effect.provideService(McpServer.McpServer, tracingServer));
1057+
});
1058+
10251059
/** `McpServer.registerToolkit` advertises the full codec schema; agents get the compact one. */
10261060
const withAdvertisedSchemas = Effect.fn("McpHttpServer.withAdvertisedSchemas")(function* <
10271061
Tools extends Record<string, Tool.Any>,
@@ -1074,7 +1108,9 @@ export const toolkitRegistration = <Tools extends Record<string, Tool.Any>, EX,
10741108
) =>
10751109
Layer.effectDiscard(
10761110
withToolAnalytics(
1077-
withAdvertisedSchemas(toolkit, McpServer.registerToolkit(decodeAsAdvertised(toolkit))),
1111+
withToolSpans(
1112+
withAdvertisedSchemas(toolkit, McpServer.registerToolkit(decodeAsAdvertised(toolkit))),
1113+
),
10781114
),
10791115
).pipe(
10801116
Layer.provide(
@@ -1090,7 +1126,7 @@ const imageToolRegistration = <Tools extends Record<string, Tool.Any>, A, E, R,
10901126
register: Effect.Effect<A, E, R>,
10911127
handlers: McpToolAccess.HandlersLayer<Tools, EX, RX>,
10921128
) =>
1093-
Layer.effectDiscard(withToolAnalytics(register)).pipe(
1129+
Layer.effectDiscard(withToolAnalytics(withToolSpans(register))).pipe(
10941130
Layer.provide(McpToolAccess.HandlersLayer.layer(handlers)),
10951131
);
10961132

‎docs/operations/observability.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,11 @@ span spent waiting for that connection, behind other SQL. That wait does not blo
6363
so subtract it before you blame a slow span on the query. Statements inside a transaction use its
6464
connection and do not carry these attributes.
6565

66+
An MCP request over HTTP has a `McpHttpServer.authenticateRequest` span that covers only admitting
67+
it: the credential check, the mode limit, and the Linked environments gate. The request runs after
68+
it, under the `McpServer.…/<method>` span, and each tool call has its own `mcp.tool.<name>` span
69+
that times it. That span's attributes name the tool, never its arguments.
70+
6671
DPoP proof failures include the safe `environment.dpop.failure_code` span
6772
attribute. A `time_window` failure means that a signed proof was too old or too
6873
far in the future for the environment server's allowed window. It can point to

0 commit comments

Comments
 (0)