Skip to content

Commit b34f192

Browse files
PR Review
- Trim verbose comment in browser workflow example. - Use `yarn start:mcp-demo` shortcut in demo README. - Warn once on concurrent BrowserMcpRequestContext.run(). - Feature-detect transferable ReadableStream in McpWorkerBridge; buffer to ArrayBuffer (with warn-once) on browsers without support.
1 parent 544b7f8 commit b34f192

4 files changed

Lines changed: 48 additions & 14 deletions

File tree

examples/workflow-server-mcp-demo/README.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,14 @@ The Node path is additionally covered by the automated end-to-end spec at
3232
From the repository root:
3333

3434
```bash
35-
yarn workspace @eclipse-glsp-examples/workflow-server build
36-
yarn workspace @eclipse-glsp-examples/workflow-server-mcp-demo start
35+
yarn start:mcp-demo
3736
```
3837

39-
The `start` script copies the worker bundle from
38+
This builds the workflow server, copies the worker bundle from
4039
`@eclipse-glsp-examples/workflow-server-bundled-web`, builds the page-side bundle, and serves
4140
the directory on `http://localhost:8000/`.
4241

43-
Open `http://localhost:8000/` in a Chromium-based browser. Step through the buttons
42+
Open `http://localhost:8000/` in any modern browser. Step through the buttons
4443
top-to-bottom; the workflow auto-renders once MCP is initialized, and **Create task**
4544
mutates the live session.
4645

examples/workflow-server/src/browser/app.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,7 @@ import { WorkflowDiagramModule, WorkflowServerModule } from '../common/workflow-
2525
import { WorkflowMockModelStorage } from './mock-model-storage';
2626

2727
export async function launch(_argv?: string[]): Promise<void> {
28-
// Bridge must be created before any await so postMessages that arrive on the next event-loop
29-
// tick aren't dropped. Requests pile up on the bridge's internal `launcherReady` until the
30-
// GLSP client connects and the DI container activates the MCP launcher.
28+
// Bridge must be created before any await so postMessages that arrive on the next event-loop tick aren't dropped.
3129
const bridge = new McpWorkerBridge();
3230

3331
const appContainer = new Container();

packages/server-mcp/src/browser/server/browser-mcp-request-context.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,23 @@ import { injectable } from 'inversify';
1818
import { McpRequestContext, McpRequestExtra } from '../../common/server/mcp-request-context';
1919

2020
/**
21-
* Browser-compatible {@link McpRequestContext}. Single mutable slot — concurrent requests on
22-
* the same session overwrite each other's frame. Hosts must serialise handler chains.
21+
* Browser-compatible {@link McpRequestContext} that holds one request context at a time.
22+
* Hosts must serialise dispatches so they don't overwrite each other (e.g. via {@link McpWorkerBridge}).
2323
*/
2424
@injectable()
2525
export class BrowserMcpRequestContext implements McpRequestContext {
2626
protected current: McpRequestExtra | undefined;
27+
protected concurrencyWarned = false;
2728

2829
run<R>(extra: McpRequestExtra, callback: () => R): R {
30+
// Warn once if an adopter forgot to serialise dispatches — silent context corruption would be hard to debug.
31+
if (this.current !== undefined && !this.concurrencyWarned) {
32+
this.concurrencyWarned = true;
33+
console.warn(
34+
'BrowserMcpRequestContext: concurrent run() detected — request contexts will overwrite each other. ' +
35+
'Serialise MCP dispatches (e.g. via McpWorkerBridge or an adopter-side queue around launcher.handleRequest).'
36+
);
37+
}
2938
const prior = this.current;
3039
this.current = extra;
3140
let result: R;

packages/server-mcp/src/browser/server/mcp-worker-bridge.ts

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ export interface McpResponseMessage {
3838
status: number;
3939
statusText: string;
4040
headers: Record<string, string>;
41-
/** Transferred across `postMessage` so streaming (SSE) bodies don't get buffered. */
42-
body: ReadableStream<Uint8Array> | null;
41+
/** `ReadableStream` on Chromium (streams SSE); buffered `ArrayBuffer` on browsers without transferable streams. */
42+
body: ReadableStream<Uint8Array> | ArrayBuffer | null;
4343
}
4444

4545
/** Carries the SW↔Worker `MessagePort` transfer that wires MCP traffic over a dedicated channel. */
@@ -55,6 +55,17 @@ export function isMcpInitPortMessage(value: unknown): value is McpInitPortMessag
5555
return Object(value) === value && (value as { type?: unknown }).type === MCP_INIT_PORT_MESSAGE_TYPE;
5656
}
5757

58+
/** Probes whether the current realm can transfer `ReadableStream` via `postMessage` (Chromium yes; Firefox / Safari no). */
59+
export function canTransferReadableStream(): boolean {
60+
try {
61+
const probe = new ReadableStream();
62+
structuredClone(probe, { transfer: [probe] });
63+
return true;
64+
} catch {
65+
return false;
66+
}
67+
}
68+
5869
/** Minimal Worker scope contract — defined locally so the package compiles without the `webworker` lib. */
5970
export interface McpBridgeScope {
6071
addEventListener(type: 'message', listener: (event: MessageEvent) => void): void;
@@ -73,6 +84,8 @@ export class McpWorkerBridge implements Disposable {
7384
protected readonly listener = (event: MessageEvent): void => this.onMessage(event);
7485
/** Serialise dispatches — `BrowserMcpRequestContext` is a single-slot store, not concurrency-safe. */
7586
protected dispatchChain: Promise<void> = Promise.resolve();
87+
protected readonly streamTransferable: boolean = canTransferReadableStream();
88+
protected bufferingWarned = false;
7689

7790
constructor(protected readonly scope: McpBridgeScope = self as unknown as McpBridgeScope) {
7891
this.launcherReady = new Promise<AbstractMcpServerLauncher>(resolve => {
@@ -146,7 +159,7 @@ export class McpWorkerBridge implements Disposable {
146159
status: response.status,
147160
statusText: response.statusText,
148161
headers,
149-
body: response.body
162+
body: await this.encodeBody(response)
150163
});
151164
} catch (err: unknown) {
152165
const message = err instanceof Error ? err.message : String(err);
@@ -156,13 +169,28 @@ export class McpWorkerBridge implements Disposable {
156169
status: 500,
157170
statusText: 'Internal Server Error',
158171
headers: { 'content-type': 'text/plain' },
159-
body: new Response(`MCP worker bridge error: ${message}`).body
172+
body: await this.encodeBody(new Response(`MCP worker bridge error: ${message}`))
160173
});
161174
}
162175
}
163176

177+
/** Stream on Chromium; buffer to `ArrayBuffer` (still transferable) on browsers without transferable streams. */
178+
protected async encodeBody(response: Response): Promise<ReadableStream<Uint8Array> | ArrayBuffer | null> {
179+
if (this.streamTransferable) {
180+
return response.body;
181+
}
182+
if (!this.bufferingWarned) {
183+
this.bufferingWarned = true;
184+
console.warn(
185+
'McpWorkerBridge: ReadableStream is not transferable in this browser — buffering MCP response bodies. ' +
186+
'Chunked / SSE streaming will deliver as a single chunk; non-streaming MCP calls are unaffected.'
187+
);
188+
}
189+
return response.arrayBuffer();
190+
}
191+
164192
protected reply(port: MessagePort | undefined, message: McpResponseMessage): void {
165-
const transfer = message.body ? [message.body] : [];
193+
const transfer: Transferable[] = message.body ? [message.body] : [];
166194
if (port) {
167195
port.postMessage(message, transfer);
168196
} else {

0 commit comments

Comments
 (0)