From b0cc1c75c87bb78164cba602a4ee7de027f3fdbd Mon Sep 17 00:00:00 2001 From: Guillaume ARM Date: Wed, 10 Jun 2026 22:18:05 +0200 Subject: [PATCH] test(mcp): add bridge integration probes --- docs/adrs/adr-0016-js-tool-verification.md | 18 +-- tools/mcp-bridge/eslint.config.js | 2 +- tools/mcp-bridge/package.json | 6 +- tools/mcp-bridge/test-integration/harness.ts | 151 ++++++++++++++++++ .../test-integration/lua/echo-client.lua | 42 +++++ .../lua/multi-echo-client.lua | 43 +++++ .../test-integration/lua/silent-client.lua | 27 ++++ .../test-integration/probe-empty.test.ts | 12 ++ .../test-integration/probe-happy.test.ts | 21 +++ .../test-integration/probe-multi.test.ts | 25 +++ .../test-integration/probe-silent.test.ts | 21 +++ tools/mcp-bridge/tsconfig.json | 2 +- 12 files changed, 356 insertions(+), 14 deletions(-) create mode 100644 tools/mcp-bridge/test-integration/harness.ts create mode 100644 tools/mcp-bridge/test-integration/lua/echo-client.lua create mode 100644 tools/mcp-bridge/test-integration/lua/multi-echo-client.lua create mode 100644 tools/mcp-bridge/test-integration/lua/silent-client.lua create mode 100644 tools/mcp-bridge/test-integration/probe-empty.test.ts create mode 100644 tools/mcp-bridge/test-integration/probe-happy.test.ts create mode 100644 tools/mcp-bridge/test-integration/probe-multi.test.ts create mode 100644 tools/mcp-bridge/test-integration/probe-silent.test.ts diff --git a/docs/adrs/adr-0016-js-tool-verification.md b/docs/adrs/adr-0016-js-tool-verification.md index cdfe8ea..137599a 100644 --- a/docs/adrs/adr-0016-js-tool-verification.md +++ b/docs/adrs/adr-0016-js-tool-verification.md @@ -18,11 +18,11 @@ The bridge also needs future end-to-end coverage that spans both runtimes: a hos Keep the Node package scripts simple and one-purpose: -- `npm run build` emits TypeScript into `dist/`. -- `npm run test` runs only the already-built Node unit tests. +- `npm run build` emits TypeScript into `dist/` and acts as the type-check gate. +- `npm run test` runs the Node unit tests directly from TypeScript with `tsx --test test/*.test.ts`. No prior build is required. - `npm run check` runs ESLint. TypeScript compilation is covered by `npm run build` and `npm run ci` to avoid duplicate compiler runs in repository CI. -- `npm run ci` runs `npm run build && npm run test`. -- `npm run test-integration` is reserved for bridge-to-CraftOS integration coverage and currently prints an explicit TODO. +- `npm run ci` runs `npm run check && npm run build && npm run test`, so type errors and lint failures both surface even though `test` itself no longer compiles. +- `npm run test-integration` runs the bridge-to-CraftOS integration suite with `tsx --test --test-concurrency=1 test-integration/*.test.ts`. Each case boots the bridge in-process on fixed loopback ports (`127.0.0.1:2000` for MCP HTTP, `127.0.0.1:2001` for the CraftOS link), spawns a CraftOS-PC headless computer that connects back, exercises `tools/call probe-computers`, and tears everything down. `--test-concurrency=1` keeps the fixed ports collision-free. Expose matching repository recipes for the Node lifecycle: @@ -34,12 +34,12 @@ Expose matching repository recipes for the Node lifecycle: ## Consequences -- `npm run test` no longer hides a build step. Callers that need a fresh build must use `npm run ci` or `just test`. +- `npm run test` no longer needs `dist/`. Both unit and integration tests load TypeScript directly through `tsx`, so test iteration is faster and `dist/` is only required for `npm start`. +- TypeScript errors are no longer caught by `npm run test`; they are caught by `npm run build`, which stays wired into `npm run ci`, `just build`, `just test`, and `just ci`. - `just ci` avoids duplicating Node unit tests by calling `npm run ci` directly and then invoking only the CraftOS-side test body. -- ESLint failures are part of `just check`, so they are covered by the same pre-commit and pre-push hooks as Lua and Markdown checks. TypeScript compiler failures are covered by `just build`, `just test`, and `just ci`. -- Future bridge integration tests have a named home before they exist, reducing the chance that slow end-to-end behavior is mixed into fast unit tests. +- ESLint failures are part of `just check`, so they are covered by the same pre-commit and pre-push hooks as Lua and Markdown checks. +- Integration tests live in `tools/mcp-bridge/test-integration/`, with Lua client fixtures under `test-integration/lua/`. Slow end-to-end behavior stays out of the fast unit-test path. ## Future Work -- Replace the `npm run test-integration` placeholder with a harness that launches the bridge and CraftOS-PC headless, connects a websocket client from CraftOS, probes through the MCP surface, and tears down both processes reliably. -- Give the bridge integration harness host-level timeout handling and readable diagnostics modelled on the CraftOS-PC test recipes. +- Extend the integration harness with disconnect and reconnect scenarios (computer drops mid-probe; bridge restarts while a computer is connected) once those failure modes need regression coverage. diff --git a/tools/mcp-bridge/eslint.config.js b/tools/mcp-bridge/eslint.config.js index a975adc..fae6796 100644 --- a/tools/mcp-bridge/eslint.config.js +++ b/tools/mcp-bridge/eslint.config.js @@ -18,7 +18,7 @@ export default tseslint.config( }, }, { - files: ["test/**/*.ts"], + files: ["test/**/*.ts", "test-integration/**/*.ts"], rules: { "@typescript-eslint/no-floating-promises": "off", }, diff --git a/tools/mcp-bridge/package.json b/tools/mcp-bridge/package.json index 8cfc87f..0a8c581 100644 --- a/tools/mcp-bridge/package.json +++ b/tools/mcp-bridge/package.json @@ -6,11 +6,11 @@ "scripts": { "build": "tsc --noEmit false", "check": "eslint .", - "ci": "npm run build && npm run test", + "ci": "npm run check && npm run build && npm run test", "dev": "tsx src/index.ts", "start": "node dist/src/index.js", - "test": "node --test dist/test/*.test.js", - "test-integration": "node -e \"console.log('TODO: add mcp-bridge integration tests that launch CraftOS-PC headless, start the bridge, connect a ComputerCraft websocket client from inside CraftOS, and exercise the MCP probe path end-to-end. The intended harness should verify startup ordering, websocket hello/response framing, timeout behavior when a computer is silent, and cleanup of both the CraftOS process and Node bridge server. Keep this separate from unit tests because it will need host ports, a CraftOS watchdog, and careful failure diagnostics similar to the existing Lua timeout fixtures.')\"" + "test": "tsx --test test/*.test.ts", + "test-integration": "tsx --test --test-concurrency=1 test-integration/*.test.ts" }, "dependencies": { "ws": "^8.17.1" diff --git a/tools/mcp-bridge/test-integration/harness.ts b/tools/mcp-bridge/test-integration/harness.ts new file mode 100644 index 0000000..9657270 --- /dev/null +++ b/tools/mcp-bridge/test-integration/harness.ts @@ -0,0 +1,151 @@ +import { spawn } from "node:child_process"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { fileURLToPath } from "node:url"; +import type { Server } from "node:http"; +import type { WebSocketServer } from "ws"; +import { LinkRegistry, startLinkServer } from "../src/link-server.js"; +import { startMcpServer } from "../src/mcp-server.js"; + +const HERE = dirname(fileURLToPath(import.meta.url)); +const LUA_DIR = join(HERE, "lua"); + +export const MCP_PORT = 2000; +export const LINK_PORT = 2001; +export const MCP_URL = `http://127.0.0.1:${MCP_PORT}`; + +export type Bridge = { + registry: LinkRegistry; + close: () => Promise; +}; + +export async function startBridge(probeTimeoutMs = 500): Promise { + const registry = new LinkRegistry(); + const mcpServer = startMcpServer({ host: "127.0.0.1", port: MCP_PORT, probeTimeoutMs, registry }); + const linkServer = startLinkServer({ host: "127.0.0.1", port: LINK_PORT, registry }); + await Promise.all([waitForListening(mcpServer), waitForListening(linkServer)]); + return { + registry, + close: () => closeBridge(mcpServer, linkServer), + }; +} + +export async function waitForComputers(registry: LinkRegistry, count: number, timeoutMs: number): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (registry.count() >= count) { + return; + } + await sleep(50); + } + throw new Error(`waitForComputers timed out (expected ${count}, got ${registry.count()})`); +} + +export async function callProbeComputers(): Promise { + const body = { + jsonrpc: "2.0", + id: 1, + method: "tools/call", + params: { name: "probe-computers", arguments: {} }, + }; + const response = await fetch(MCP_URL, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(body), + }); + const payload = (await response.json()) as { + result?: { content?: { type: string; text: string }[] }; + }; + const text = payload.result?.content?.[0]?.text; + if (typeof text !== "string") { + throw new Error(`Unexpected MCP response: ${JSON.stringify(payload)}`); + } + return text; +} + +export type CraftosResult = { + status: number | null; + signal: NodeJS.Signals | null; + output: string; +}; + +export type CraftosHandle = { + done: Promise; + abort: () => void; +}; + +export function startCraftos( + luaName: string, + opts: { shellArgs?: string[]; timeoutMs?: number } = {}, +): CraftosHandle { + const timeoutMs = opts.timeoutMs ?? 15_000; + const controller = new AbortController(); + + const done = (async (): Promise => { + const dataDir = await mkdtemp(join(tmpdir(), "mcp-bridge-it-")); + const watchdog = setTimeout(() => controller.abort(), timeoutMs); + try { + const args: string[] = ["--directory", dataDir, "--headless", "--mount-ro", `/staging=${LUA_DIR}`]; + if (process.platform === "darwin") { + args.push("--rom", "/Applications/CraftOS-PC.app/Contents/Resources"); + } + args.push("--exec", buildExecCode(luaName, opts.shellArgs ?? [])); + + const chunks: Buffer[] = []; + const child = spawn("craftos", args, { signal: controller.signal }); + child.stdout.on("data", (d: Buffer) => chunks.push(d)); + child.stderr.on("data", (d: Buffer) => chunks.push(d)); + + const result = await new Promise<{ status: number | null; signal: NodeJS.Signals | null }>((resolve) => { + child.once("close", (code, signal) => resolve({ status: code, signal })); + child.once("error", () => resolve({ status: null, signal: null })); + }); + + return { status: result.status, signal: result.signal, output: Buffer.concat(chunks).toString("utf8") }; + } finally { + clearTimeout(watchdog); + await rm(dataDir, { recursive: true, force: true }); + } + })(); + + return { done, abort: () => controller.abort() }; +} + +export function formatFailure(message: string, craftosOutput: string): string { + const lines = ["\x1b[31mFAIL\x1b[0m " + message, "--- craftos output ---", craftosOutput.trimEnd(), "----------------------"]; + return lines.join("\n"); +} + +function buildExecCode(luaName: string, shellArgs: string[]): string { + const parts = [`'/staging/${luaName}'`, ...shellArgs.map(luaQuote)]; + return `shell.run(${parts.join(", ")})`; +} + +function luaQuote(value: string): string { + return `'${value.replaceAll("\\", "\\\\").replaceAll("'", "\\'")}'`; +} + +function waitForListening(server: Server | WebSocketServer): Promise { + return new Promise((resolve, reject) => { + const addr = "address" in server ? server.address() : null; + if (addr) { + resolve(); + return; + } + server.once("listening", () => resolve()); + server.once("error", reject); + }); +} + +async function closeBridge(mcpServer: Server, linkServer: WebSocketServer): Promise { + for (const client of linkServer.clients) { + client.terminate(); + } + await new Promise((resolve) => linkServer.close(() => resolve())); + await new Promise((resolve) => mcpServer.close(() => resolve())); +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} diff --git a/tools/mcp-bridge/test-integration/lua/echo-client.lua b/tools/mcp-bridge/test-integration/lua/echo-client.lua new file mode 100644 index 0000000..b02c108 --- /dev/null +++ b/tools/mcp-bridge/test-integration/lua/echo-client.lua @@ -0,0 +1,42 @@ +-- Integration-test helper: connect to the mcp-bridge link server, send a hello +-- frame, then reply to every `request` with a `pong` response until a deadline +-- expires. Args (via shell.run): id, label, urlBase, durationSeconds. +local args = {...}; + +local id = tonumber(args[1]) or 1; +local label = args[2] or "echo"; +local urlBase = args[3] or "ws://127.0.0.1:2001"; +local duration = tonumber(args[4]) or 5; + +local url = urlBase .. "/?id=" .. id; +local ws, err = http.websocket(url); +if not ws then + print("websocket failed: " .. tostring(err)); + os.shutdown(); + return; +end + +ws.send(textutils.serializeJSON({ + type = "hello", + computerId = id, + computerLabel = label, +})); + +local deadline = os.epoch("utc") + duration * 1000; +while os.epoch("utc") < deadline do + local msg = ws.receive(0.5); + if msg then + local frame = textutils.unserializeJSON(msg); + if frame and frame.type == "request" then + ws.send(textutils.serializeJSON({ + type = "response", + id = frame.id, + ok = true, + result = "pong from " .. id .. " (Label: " .. label .. ")", + })); + end + end +end + +pcall(function() ws.close(); end); +os.shutdown(); diff --git a/tools/mcp-bridge/test-integration/lua/multi-echo-client.lua b/tools/mcp-bridge/test-integration/lua/multi-echo-client.lua new file mode 100644 index 0000000..fa1b7c7 --- /dev/null +++ b/tools/mcp-bridge/test-integration/lua/multi-echo-client.lua @@ -0,0 +1,43 @@ +-- Integration-test helper: open two websocket connections in parallel, each +-- presenting itself as a distinct logical computer. Used by probe-multi. +local urlBase = "ws://127.0.0.1:2001"; + +local function connect(id, label, duration) + local url = urlBase .. "/?id=" .. id; + local ws, err = http.websocket(url); + if not ws then + print("websocket failed (" .. id .. "): " .. tostring(err)); + return; + end + + ws.send(textutils.serializeJSON({ + type = "hello", + computerId = id, + computerLabel = label, + })); + + local deadline = os.epoch("utc") + duration * 1000; + while os.epoch("utc") < deadline do + local msg = ws.receive(0.3); + if msg then + local frame = textutils.unserializeJSON(msg); + if frame and frame.type == "request" then + ws.send(textutils.serializeJSON({ + type = "response", + id = frame.id, + ok = true, + result = "pong from " .. id .. " (Label: " .. label .. ")", + })); + end + end + end + + pcall(function() ws.close(); end); +end + +parallel.waitForAll( + function() connect(1001, "echo-A", 5); end, + function() connect(1002, "echo-B", 5); end +); + +os.shutdown(); diff --git a/tools/mcp-bridge/test-integration/lua/silent-client.lua b/tools/mcp-bridge/test-integration/lua/silent-client.lua new file mode 100644 index 0000000..7b284e7 --- /dev/null +++ b/tools/mcp-bridge/test-integration/lua/silent-client.lua @@ -0,0 +1,27 @@ +-- Integration-test helper: connect, send a hello frame, then ignore every +-- request until the deadline expires. Used to prove the bridge's per-computer +-- probe timeout path. +local args = {...}; + +local id = tonumber(args[1]) or 99; +local label = args[2] or "silent"; +local duration = tonumber(args[3]) or 5; + +local url = "ws://127.0.0.1:2001/?id=" .. id; +local ws, err = http.websocket(url); +if not ws then + print("websocket failed: " .. tostring(err)); + os.shutdown(); + return; +end + +ws.send(textutils.serializeJSON({ + type = "hello", + computerId = id, + computerLabel = label, +})); + +sleep(duration); + +pcall(function() ws.close(); end); +os.shutdown(); diff --git a/tools/mcp-bridge/test-integration/probe-empty.test.ts b/tools/mcp-bridge/test-integration/probe-empty.test.ts new file mode 100644 index 0000000..0020751 --- /dev/null +++ b/tools/mcp-bridge/test-integration/probe-empty.test.ts @@ -0,0 +1,12 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { callProbeComputers, startBridge } from "./harness.js"; + +test("probe-computers returns the no-computers message when nothing is connected", async () => { + const bridge = await startBridge(); + try { + assert.equal(await callProbeComputers(), "No computers connected."); + } finally { + await bridge.close(); + } +}); diff --git a/tools/mcp-bridge/test-integration/probe-happy.test.ts b/tools/mcp-bridge/test-integration/probe-happy.test.ts new file mode 100644 index 0000000..da9afbe --- /dev/null +++ b/tools/mcp-bridge/test-integration/probe-happy.test.ts @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { callProbeComputers, formatFailure, startBridge, startCraftos, waitForComputers } from "./harness.js"; + +test("probe-computers aggregates a single CraftOS echo computer", async () => { + const bridge = await startBridge(); + const craftos = startCraftos("echo-client.lua", { shellArgs: ["1", "echo-1", "ws://127.0.0.1:2001", "8"] }); + try { + await waitForComputers(bridge.registry, 1, 12_000); + const text = await callProbeComputers(); + assert.equal(text, "pong from 1 (Label: echo-1)"); + } catch (error) { + craftos.abort(); + const result = await craftos.done; + throw new Error(formatFailure(error instanceof Error ? error.message : String(error), result.output), { cause: error }); + } finally { + craftos.abort(); + await craftos.done; + await bridge.close(); + } +}); diff --git a/tools/mcp-bridge/test-integration/probe-multi.test.ts b/tools/mcp-bridge/test-integration/probe-multi.test.ts new file mode 100644 index 0000000..e1d4045 --- /dev/null +++ b/tools/mcp-bridge/test-integration/probe-multi.test.ts @@ -0,0 +1,25 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { callProbeComputers, formatFailure, startBridge, startCraftos, waitForComputers } from "./harness.js"; + +test("probe-computers aggregates two CraftOS computers from the same headless process", async () => { + const bridge = await startBridge(); + const craftos = startCraftos("multi-echo-client.lua", { timeoutMs: 15_000 }); + try { + await waitForComputers(bridge.registry, 2, 12_000); + const text = await callProbeComputers(); + const lines = text.split("\n").sort(); + assert.deepEqual(lines, [ + "pong from 1001 (Label: echo-A)", + "pong from 1002 (Label: echo-B)", + ]); + } catch (error) { + craftos.abort(); + const result = await craftos.done; + throw new Error(formatFailure(error instanceof Error ? error.message : String(error), result.output), { cause: error }); + } finally { + craftos.abort(); + await craftos.done; + await bridge.close(); + } +}); diff --git a/tools/mcp-bridge/test-integration/probe-silent.test.ts b/tools/mcp-bridge/test-integration/probe-silent.test.ts new file mode 100644 index 0000000..98ed395 --- /dev/null +++ b/tools/mcp-bridge/test-integration/probe-silent.test.ts @@ -0,0 +1,21 @@ +import assert from "node:assert/strict"; +import test from "node:test"; +import { callProbeComputers, formatFailure, startBridge, startCraftos, waitForComputers } from "./harness.js"; + +test("probe-computers reports timeout for a connected computer that never replies", async () => { + const bridge = await startBridge(300); + const craftos = startCraftos("silent-client.lua", { shellArgs: ["7", "silent-7", "8"] }); + try { + await waitForComputers(bridge.registry, 1, 12_000); + const text = await callProbeComputers(); + assert.equal(text, "timeout from 7 (Label: silent-7)"); + } catch (error) { + craftos.abort(); + const result = await craftos.done; + throw new Error(formatFailure(error instanceof Error ? error.message : String(error), result.output), { cause: error }); + } finally { + craftos.abort(); + await craftos.done; + await bridge.close(); + } +}); diff --git a/tools/mcp-bridge/tsconfig.json b/tools/mcp-bridge/tsconfig.json index 005d2e7..1465a73 100644 --- a/tools/mcp-bridge/tsconfig.json +++ b/tools/mcp-bridge/tsconfig.json @@ -11,5 +11,5 @@ "rootDir": ".", "declaration": true }, - "include": ["src/**/*.ts", "test/**/*.ts"] + "include": ["src/**/*.ts", "test/**/*.ts", "test-integration/**/*.ts"] }