mirror of
https://github.com/morluto/rea.git
synced 2026-10-26 17:35:11 +00:00
perf(test): reuse one stdio MCP server per JavaScript application file (#5658)
* perf(test): reuse one stdio MCP server per JavaScript application file Repeated analyzeJavaScriptDirectory MCP cases started a full server for every case. Share one file-scoped server and compile cache so later cases skip that startup while CLI cases still launch a fresh process. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(test): close in-flight shared MCP startup with the file File teardown waits for a stdio session that is still starting, skips a server once close has begun, and still closes one registered during that wait. The later-case reuse test can run without the earlier case. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(test): keep a failed MCP handshake when cleanup fails The shared session reports the startup error and carries the cleanup failure as its cause. A cleanup that failed stays owned and is retried on close without running a cleanup that already succeeded. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(test): fail shared MCP cleanup when the rejection has no reason A cleanup rejection of undefined still fails the drain and stays owned. Close caches completion only after every retained cleanup succeeds, so a later close can release it. Co-authored-by: morluto <morluto@users.noreply.github.com> * fix(test): keep one MCP server while failed cleanup is retained A later call retries retained cleanup before opening another server. The new server starts only after that cleanup succeeds. Co-authored-by: morluto <morluto@users.noreply.github.com> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: morluto <morluto@users.noreply.github.com>
This commit is contained in:
4 files changed
+473
-7
No files matched your search
@@ -146,6 +146,17 @@ the fixture cache. These subprocesses are outside aggregate Vitest coverage;
|
||||
subprocess coverage attachment remains disabled. Cold-start import checks launch
|
||||
independently without this fixture cache.
|
||||
|
||||
`analyzeJavaScriptDirectory` stdio MCP cases in one `cliTest` file share one
|
||||
server and that compile cache. The first case pays startup; later cases reuse
|
||||
the process. File teardown waits for startup still in flight, closes a server
|
||||
registered during that wait, and does not open a server after close. A failed
|
||||
handshake stays the caller error when its cleanup also fails, including a
|
||||
rejection without a reason. That cleanup stays owned until a later close or
|
||||
call releases it. Another server does not open while the cleanup remains, and
|
||||
a cleanup that already succeeded is not run again. Pass
|
||||
`mcpCall` when a case needs its own server, environment, or lifecycle. CLI
|
||||
cases still start a fresh process.
|
||||
|
||||
The Apple artifact verifier step shares a job-temporary bytecode cache among
|
||||
its fresh CLI/MCP children via `REA_ARTIFACT_NODE_COMPILE_CACHE`. The artifact
|
||||
helper maps this setting to child `NODE_COMPILE_CACHE`; npm, Turbo, and nested
|
||||
|
||||
@@ -0,0 +1,264 @@
|
||||
import { expect, test } from "vitest";
|
||||
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import {
|
||||
cliTest,
|
||||
createSharedMcpSession,
|
||||
type SharedMcpCall,
|
||||
type TestCli,
|
||||
} from "../../support/cli/cliFixture.js";
|
||||
import {
|
||||
analyzeJavaScriptDirectory,
|
||||
resolveJavaScriptApplicationMcpCall,
|
||||
} from "../../support/cli/javascriptApplication.js";
|
||||
import { writeFixtureFiles } from "../../support/workspace/writeFixtureFiles.js";
|
||||
|
||||
const unusedCli = {
|
||||
run: async () => {
|
||||
throw new Error("CLI was not requested");
|
||||
},
|
||||
} as TestCli;
|
||||
|
||||
test("prefers an explicit JavaScript MCP caller over the file session", async () => {
|
||||
const explicit = (async () => ({
|
||||
isError: false,
|
||||
})) as unknown as SharedMcpCall;
|
||||
let sharedCalls = 0;
|
||||
const resolved = await resolveJavaScriptApplicationMcpCall(
|
||||
{
|
||||
...unusedCli,
|
||||
sharedMcpCall: async () => {
|
||||
sharedCalls += 1;
|
||||
return explicit;
|
||||
},
|
||||
},
|
||||
explicit,
|
||||
);
|
||||
expect(resolved).toBe(explicit);
|
||||
expect(sharedCalls).toBe(0);
|
||||
});
|
||||
|
||||
test("uses the file session when the case does not own a server", async () => {
|
||||
const shared = (async () => ({ isError: false })) as unknown as SharedMcpCall;
|
||||
const resolved = await resolveJavaScriptApplicationMcpCall(
|
||||
{
|
||||
...unusedCli,
|
||||
sharedMcpCall: async () => shared,
|
||||
},
|
||||
undefined,
|
||||
);
|
||||
expect(resolved).toBe(shared);
|
||||
});
|
||||
|
||||
const writeApplication = async (prefix: string): Promise<string> => {
|
||||
const root = await createTestTempDirectory(prefix);
|
||||
await writeFixtureFiles(root, {
|
||||
"package.json": JSON.stringify({ main: "main.cjs" }),
|
||||
"main.cjs":
|
||||
'const { BrowserWindow } = require("electron"); new BrowserWindow().loadFile("index.html");',
|
||||
"index.html": "page",
|
||||
});
|
||||
return root;
|
||||
};
|
||||
|
||||
let fileSession: SharedMcpCall | undefined;
|
||||
|
||||
/** Record the first selected case and compare later cases when they also run. */
|
||||
const useFileSession = async (cli: TestCli): Promise<SharedMcpCall> => {
|
||||
const open = cli.sharedMcpCall;
|
||||
if (open === undefined)
|
||||
throw new Error("cliTest did not provide a shared MCP session");
|
||||
const current = await open();
|
||||
if (fileSession === undefined) fileSession = current;
|
||||
else expect(current).toBe(fileSession);
|
||||
return current;
|
||||
};
|
||||
|
||||
const held = (): { gate: Promise<void>; release: () => void } => {
|
||||
let release: () => void = () => {};
|
||||
const gate = new Promise<void>((resolve) => {
|
||||
release = resolve;
|
||||
});
|
||||
return { gate, release };
|
||||
};
|
||||
|
||||
const fakeCall = (async () => ({
|
||||
isError: false,
|
||||
})) as unknown as SharedMcpCall;
|
||||
|
||||
test("does not open a shared MCP server after file cleanup starts", async () => {
|
||||
const startup = held();
|
||||
let opened = false;
|
||||
const session = createSharedMcpSession(async (_registerCleanup, isClosed) => {
|
||||
await startup.gate;
|
||||
if (isClosed()) throw new Error("Shared MCP session is closed");
|
||||
opened = true;
|
||||
return fakeCall;
|
||||
});
|
||||
const opening = session.session.call();
|
||||
const closing = session.close();
|
||||
startup.release();
|
||||
await closing;
|
||||
await expect(opening).rejects.toThrow("Shared MCP session is closed");
|
||||
expect(opened).toBe(false);
|
||||
await expect(session.session.call()).rejects.toThrow(
|
||||
"Shared MCP session is closed",
|
||||
);
|
||||
});
|
||||
|
||||
test("keeps a failed MCP handshake when its cleanup also fails", async () => {
|
||||
const startup = new Error("handshake failed");
|
||||
const cleanup = new Error("teardown failed");
|
||||
let released = 0;
|
||||
let retained = 0;
|
||||
const session = createSharedMcpSession(async (registerCleanup) => {
|
||||
registerCleanup(async () => {
|
||||
retained += 1;
|
||||
if (retained === 1) throw cleanup;
|
||||
});
|
||||
registerCleanup(async () => {
|
||||
released += 1;
|
||||
});
|
||||
throw startup;
|
||||
});
|
||||
const caught = await session.session.call().then(
|
||||
() => {
|
||||
throw new Error("startup succeeded");
|
||||
},
|
||||
(error: unknown) => error,
|
||||
);
|
||||
expect(caught).toBeInstanceOf(Error);
|
||||
if (!(caught instanceof Error)) throw new Error("expected startup failure");
|
||||
expect(caught.message).toBe(startup.message);
|
||||
expect(caught.cause).toBeInstanceOf(AggregateError);
|
||||
const cause = caught.cause as AggregateError;
|
||||
expect(cause.cause).toBe(startup);
|
||||
expect([...cause.errors]).toEqual([startup, cleanup]);
|
||||
expect(released).toBe(1);
|
||||
expect(retained).toBe(1);
|
||||
await session.close();
|
||||
expect(released).toBe(1);
|
||||
expect(retained).toBe(2);
|
||||
});
|
||||
|
||||
test("retries a reasonless MCP cleanup rejection on a later close", async () => {
|
||||
const startup = new Error("handshake failed");
|
||||
let attempts = 0;
|
||||
let fail = true;
|
||||
const session = createSharedMcpSession(async (registerCleanup) => {
|
||||
registerCleanup(() => {
|
||||
attempts += 1;
|
||||
return fail ? Promise.reject() : Promise.resolve();
|
||||
});
|
||||
throw startup;
|
||||
});
|
||||
const caught = await session.session.call().then(
|
||||
() => {
|
||||
throw new Error("startup succeeded");
|
||||
},
|
||||
(error: unknown) => error,
|
||||
);
|
||||
expect(caught).toBeInstanceOf(Error);
|
||||
if (!(caught instanceof Error)) throw new Error("expected startup failure");
|
||||
expect(caught.message).toBe(startup.message);
|
||||
expect(caught.cause).toBeInstanceOf(AggregateError);
|
||||
const cause = caught.cause as AggregateError;
|
||||
expect(cause.cause).toBe(startup);
|
||||
expect([...cause.errors]).toEqual([startup, undefined]);
|
||||
await expect(session.close()).rejects.toBeUndefined();
|
||||
expect(attempts).toBe(2);
|
||||
fail = false;
|
||||
await session.close();
|
||||
expect(attempts).toBe(3);
|
||||
});
|
||||
|
||||
test("does not open another MCP server while failed cleanup is retained", async () => {
|
||||
const startup = new Error("handshake failed");
|
||||
const cleanup = new Error("teardown failed");
|
||||
let openings = 0;
|
||||
let attempts = 0;
|
||||
let failCleanup = true;
|
||||
const session = createSharedMcpSession(async (registerCleanup) => {
|
||||
openings += 1;
|
||||
registerCleanup(async () => {
|
||||
attempts += 1;
|
||||
if (failCleanup) throw cleanup;
|
||||
});
|
||||
throw startup;
|
||||
});
|
||||
const first = await session.session.call().then(
|
||||
() => {
|
||||
throw new Error("startup succeeded");
|
||||
},
|
||||
(error: unknown) => error,
|
||||
);
|
||||
expect(first).toBeInstanceOf(Error);
|
||||
if (!(first instanceof Error)) throw new Error("expected startup failure");
|
||||
expect(first.message).toBe(startup.message);
|
||||
expect(openings).toBe(1);
|
||||
expect(attempts).toBe(1);
|
||||
|
||||
const blocked = await session.session.call().then(
|
||||
() => {
|
||||
throw new Error("startup succeeded");
|
||||
},
|
||||
(error: unknown) => error,
|
||||
);
|
||||
expect(blocked).toBe(cleanup);
|
||||
expect(openings).toBe(1);
|
||||
expect(attempts).toBe(2);
|
||||
|
||||
failCleanup = false;
|
||||
const reopened = await session.session.call().then(
|
||||
() => {
|
||||
throw new Error("startup succeeded");
|
||||
},
|
||||
(error: unknown) => error,
|
||||
);
|
||||
expect(reopened).toBe(startup);
|
||||
expect(openings).toBe(2);
|
||||
expect(attempts).toBe(4);
|
||||
});
|
||||
|
||||
test("closes a shared MCP server registered after file cleanup starts", async () => {
|
||||
const startup = held();
|
||||
let closedServer = false;
|
||||
const session = createSharedMcpSession(async (registerCleanup) => {
|
||||
await startup.gate;
|
||||
registerCleanup(async () => {
|
||||
closedServer = true;
|
||||
});
|
||||
return fakeCall;
|
||||
});
|
||||
const opening = session.session.call();
|
||||
const closing = session.close();
|
||||
startup.release();
|
||||
await closing;
|
||||
await expect(opening).rejects.toThrow("Shared MCP session is closed");
|
||||
expect(closedServer).toBe(true);
|
||||
});
|
||||
|
||||
cliTest(
|
||||
"opens one stdio MCP session for JavaScript application cases",
|
||||
async ({ cli }) => {
|
||||
const first = await useFileSession(cli);
|
||||
expect(await useFileSession(cli)).toBe(first);
|
||||
const root = await writeApplication("rea-shared-mcp-");
|
||||
const analyzed = await analyzeJavaScriptDirectory("stdio MCP", cli, root);
|
||||
expect(
|
||||
analyzed.graph.nodes.some(({ kind }) => kind === "electron-renderer"),
|
||||
).toBe(true);
|
||||
},
|
||||
);
|
||||
|
||||
cliTest(
|
||||
"reuses that stdio MCP session for a later case in the file",
|
||||
async ({ cli }) => {
|
||||
await useFileSession(cli);
|
||||
const root = await writeApplication("rea-shared-mcp-later-");
|
||||
const analyzed = await analyzeJavaScriptDirectory("stdio MCP", cli, root);
|
||||
expect(
|
||||
analyzed.graph.nodes.some(({ kind }) => kind === "electron-renderer"),
|
||||
).toBe(true);
|
||||
},
|
||||
);
|
||||
@@ -25,11 +25,174 @@ export interface CliResult extends ProcessResult {
|
||||
readonly json: unknown | undefined;
|
||||
}
|
||||
|
||||
/** One compiled stdio MCP tool call. */
|
||||
export type SharedMcpCall = Awaited<
|
||||
ReturnType<
|
||||
typeof import("../../fixtures/localToolsMcp.js").connectLocalToolsMcp
|
||||
>
|
||||
>["call"];
|
||||
|
||||
/** Compiled REA CLI driver owned by one test. */
|
||||
export interface TestCli {
|
||||
run(invocation: CliInvocation): Promise<CliResult>;
|
||||
/**
|
||||
* File-scoped stdio MCP caller. Present on `cliTest` drivers and absent for
|
||||
* drivers created with `createTestCli` alone.
|
||||
*/
|
||||
sharedMcpCall?(): Promise<SharedMcpCall>;
|
||||
}
|
||||
|
||||
/** One file-scoped stdio MCP session. */
|
||||
export interface SharedMcpSession {
|
||||
call(): Promise<SharedMcpCall>;
|
||||
}
|
||||
|
||||
const sharedMcpSessionClosed = (): Error =>
|
||||
new Error("Shared MCP session is closed");
|
||||
|
||||
/** Keep the handshake as the caller error and retain the cleanup failure beside it. */
|
||||
const startupFailureWithCleanup = (
|
||||
startup: unknown,
|
||||
cleanup: unknown,
|
||||
): Error => {
|
||||
const primary =
|
||||
startup instanceof Error ? startup : new Error(String(startup));
|
||||
return new Error(primary.message, {
|
||||
cause: new AggregateError([primary, cleanup], primary.message, {
|
||||
cause: primary,
|
||||
}),
|
||||
});
|
||||
};
|
||||
|
||||
/**
|
||||
* Start at most one server for the file. Close waits for startup still in
|
||||
* flight. A connector that sees `isClosed` can skip the server; a server
|
||||
* registered after close is still cleaned up. A failed cleanup stays owned,
|
||||
* and the next call does not open another server until that cleanup succeeds.
|
||||
*/
|
||||
export const createSharedMcpSession = (
|
||||
connect: (
|
||||
registerCleanup: (cleanup: () => Promise<void>) => void,
|
||||
isClosed: () => boolean,
|
||||
) => Promise<SharedMcpCall>,
|
||||
): { session: SharedMcpSession; close(): Promise<void> } => {
|
||||
let pending: Promise<SharedMcpCall> | undefined;
|
||||
let closed = false;
|
||||
let closing: Promise<void> | undefined;
|
||||
let closeGeneration: symbol | undefined;
|
||||
let cleanupQueue: Promise<void> = Promise.resolve();
|
||||
const cleanups: Array<() => Promise<void>> = [];
|
||||
const drain = (): Promise<void> => {
|
||||
const run = cleanupQueue
|
||||
.then(
|
||||
() => undefined,
|
||||
() => undefined,
|
||||
)
|
||||
.then(async () => {
|
||||
const failures: unknown[] = [];
|
||||
const retained: Array<() => Promise<void>> = [];
|
||||
while (cleanups.length > 0) {
|
||||
const cleanup = cleanups.pop();
|
||||
if (cleanup === undefined) break;
|
||||
try {
|
||||
await cleanup();
|
||||
} catch (error) {
|
||||
failures.push(error);
|
||||
retained.push(cleanup);
|
||||
}
|
||||
}
|
||||
for (const cleanup of retained.reverse()) cleanups.push(cleanup);
|
||||
// A reasonless rejection is `undefined` and still owns the cleanup.
|
||||
if (failures.length > 1) {
|
||||
throw new AggregateError(
|
||||
failures,
|
||||
"Shared MCP session cleanup failed",
|
||||
);
|
||||
}
|
||||
if (failures.length === 1) throw failures[0];
|
||||
});
|
||||
cleanupQueue = run;
|
||||
return run;
|
||||
};
|
||||
const registerCleanup = (cleanup: () => Promise<void>): void => {
|
||||
cleanups.push(cleanup);
|
||||
};
|
||||
return {
|
||||
session: {
|
||||
call: () => {
|
||||
if (closed) return Promise.reject(sharedMcpSessionClosed());
|
||||
pending ??= (async () => {
|
||||
try {
|
||||
if (cleanups.length > 0) await drain();
|
||||
} catch (cleanupError) {
|
||||
// Close owns cleanup once shutdown has started, so a failed
|
||||
// handshake cannot clear the startup close is awaiting.
|
||||
if (!closed) pending = undefined;
|
||||
throw cleanupError;
|
||||
}
|
||||
if (closed) throw sharedMcpSessionClosed();
|
||||
try {
|
||||
const opened = await connect(registerCleanup, () => closed);
|
||||
if (closed) throw sharedMcpSessionClosed();
|
||||
return opened;
|
||||
} catch (error) {
|
||||
if (!closed) {
|
||||
pending = undefined;
|
||||
try {
|
||||
await drain();
|
||||
} catch (cleanupError) {
|
||||
throw startupFailureWithCleanup(error, cleanupError);
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
})();
|
||||
return pending;
|
||||
},
|
||||
},
|
||||
close: () => {
|
||||
closed = true;
|
||||
if (closing !== undefined) return closing;
|
||||
const starting = pending;
|
||||
const generation = Symbol("shared-mcp-close");
|
||||
closeGeneration = generation;
|
||||
const attempt = (async () => {
|
||||
try {
|
||||
if (starting !== undefined) {
|
||||
await starting.catch(() => undefined);
|
||||
}
|
||||
await drain();
|
||||
} catch (error) {
|
||||
// A failed close stays retryable. Completion is cached only after
|
||||
// every retained cleanup has succeeded.
|
||||
if (closeGeneration === generation) {
|
||||
closing = undefined;
|
||||
closeGeneration = undefined;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
})();
|
||||
closing = attempt;
|
||||
return attempt;
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
const createFileMcpSession = (
|
||||
compileCache: string,
|
||||
): { session: SharedMcpSession; close(): Promise<void> } =>
|
||||
createSharedMcpSession(async (registerCleanup, isClosed) => {
|
||||
const { connectLocalToolsMcp } = await import(
|
||||
"../../fixtures/localToolsMcp.js"
|
||||
);
|
||||
if (isClosed()) throw sharedMcpSessionClosed();
|
||||
const mcp = await connectLocalToolsMcp(
|
||||
{ NODE_COMPILE_CACHE: compileCache },
|
||||
registerCleanup,
|
||||
);
|
||||
return mcp.call;
|
||||
});
|
||||
|
||||
const parseJson = (stdout: string): unknown | undefined => {
|
||||
const document = stdout.trim();
|
||||
if (document.length === 0) return undefined;
|
||||
@@ -92,6 +255,7 @@ export const createTestCli = (
|
||||
/** Vitest API extended with an owned process registry and compiled CLI driver. */
|
||||
export const cliTest = processTest.extend<{
|
||||
$test: { cli: TestCli };
|
||||
$file: { mcpSession: SharedMcpSession };
|
||||
$worker: { cliCompileCache: string };
|
||||
}>({
|
||||
cliCompileCache: [
|
||||
@@ -106,7 +270,22 @@ export const cliTest = processTest.extend<{
|
||||
},
|
||||
{ scope: "worker" },
|
||||
],
|
||||
cli: async ({ processes, cliCompileCache }, use) => {
|
||||
await use(createTestCli(processes, cliCompileCache));
|
||||
mcpSession: [
|
||||
async ({ cliCompileCache }, use) => {
|
||||
const opened = createFileMcpSession(cliCompileCache);
|
||||
try {
|
||||
await use(opened.session);
|
||||
} finally {
|
||||
await opened.close();
|
||||
}
|
||||
},
|
||||
{ scope: "file" },
|
||||
],
|
||||
cli: async ({ processes, cliCompileCache, mcpSession }, use) => {
|
||||
const cli = createTestCli(processes, cliCompileCache);
|
||||
await use({
|
||||
...cli,
|
||||
sharedMcpCall: () => mcpSession.call(),
|
||||
});
|
||||
},
|
||||
});
|
||||
@@ -6,7 +6,7 @@ import { z } from "zod";
|
||||
|
||||
import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js";
|
||||
import { connectLocalToolsMcp } from "../../fixtures/localToolsMcp.js";
|
||||
import type { TestCli } from "./cliFixture.js";
|
||||
import type { SharedMcpCall, TestCli } from "./cliFixture.js";
|
||||
|
||||
const execute = promisify(execFile);
|
||||
|
||||
@@ -21,6 +21,20 @@ export const runNodeJsonControl = async (
|
||||
return JSON.parse(stdout) as unknown;
|
||||
};
|
||||
|
||||
/**
|
||||
* Use an explicit caller when the test owns the server. Otherwise reuse the
|
||||
* file-scoped session from `cliTest`, and start a fresh server only for
|
||||
* drivers created outside that fixture.
|
||||
*/
|
||||
export const resolveJavaScriptApplicationMcpCall = async (
|
||||
cli: TestCli,
|
||||
mcpCall: SharedMcpCall | undefined,
|
||||
): Promise<SharedMcpCall> => {
|
||||
if (mcpCall !== undefined) return mcpCall;
|
||||
if (cli.sharedMcpCall !== undefined) return cli.sharedMcpCall();
|
||||
return (await connectLocalToolsMcp()).call;
|
||||
};
|
||||
|
||||
/** Analyze a directory; a supplied MCP caller reuses its test-owned connection. */
|
||||
export const analyzeJavaScriptDirectory = async (
|
||||
surface: "CLI" | "stdio MCP",
|
||||
@@ -31,9 +45,7 @@ export const analyzeJavaScriptDirectory = async (
|
||||
mcpCall,
|
||||
}: {
|
||||
readonly sourcePaths?: readonly string[];
|
||||
readonly mcpCall?:
|
||||
| Awaited<ReturnType<typeof connectLocalToolsMcp>>["call"]
|
||||
| undefined;
|
||||
readonly mcpCall?: SharedMcpCall | undefined;
|
||||
} = {},
|
||||
) => {
|
||||
let document: unknown;
|
||||
@@ -51,7 +63,7 @@ export const analyzeJavaScriptDirectory = async (
|
||||
expect(response.exitCode, response.stderr).toBe(0);
|
||||
document = response.json;
|
||||
} else {
|
||||
const call = mcpCall ?? (await connectLocalToolsMcp()).call;
|
||||
const call = await resolveJavaScriptApplicationMcpCall(cli, mcpCall);
|
||||
const response = await call("analyze_javascript_application", {
|
||||
input_path: root,
|
||||
format: "directory",
|
||||
|
||||
Reference in new issue
Block a user