From fd030b408dcdad409c5fa4f458febb27e919a065 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:24:03 -0300 Subject: [PATCH] fix(cli): add POST /api/mcp/restart and mcp enable/disable subcommands (#13012) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `bin/cli/commands/mcp.mjs::runMcpRestartCommand` POSTs to /api/mcp/restart, but that route never existed under src/app/api/mcp/ — every `omniroute mcp restart` call 404d. There was also no CLI/env path to flip the mcpEnabled setting on: it was dashboard-only (/dashboard/mcp -> PATCH /api/settings), stranding headless/portable installs (Windows portable Node, no browser session) with no way to reach the MCP server's 110 tools. Adds the missing route (auth-gated via requireManagementAuth, 409 when disabled, 501 for stdio since HTTP/SSE sessions are the only in-process handle shutdownMcpHttp() can tear down) plus `omniroute mcp enable [--transport stdio|sse|streamable-http]` / `mcp disable`, which PATCH /api/settings the same way the dashboard toggle does. `mcp status` now hints at `mcp enable` when it reports the server stopped because it is disabled. Regression tests: tests/unit/mcp-restart-route-13012.test.ts (route behavior across disabled/stdio/http-family states + LOCAL_ONLY classification) and tests/unit/cli-mcp-enable-disable-13012.test.ts (PATCH body shape). --- bin/cli/commands/mcp.mjs | 110 +++++++++++++++++- bin/cli/locales/en.json | 10 ++ bin/cli/locales/zh-CN.json | 10 ++ bin/cli/locales/zh-TW.json | 10 ++ .../fixes/13012-cli-mcp-restart-and-enable.md | 1 + docs/frameworks/MCP-SERVER.md | 17 +++ src/app/api/mcp/restart/route.ts | 53 +++++++++ .../unit/cli-mcp-enable-disable-13012.test.ts | 95 +++++++++++++++ tests/unit/mcp-restart-route-13012.test.ts | 77 ++++++++++++ 9 files changed, 377 insertions(+), 6 deletions(-) create mode 100644 changelog.d/fixes/13012-cli-mcp-restart-and-enable.md create mode 100644 src/app/api/mcp/restart/route.ts create mode 100644 tests/unit/cli-mcp-enable-disable-13012.test.ts create mode 100644 tests/unit/mcp-restart-route-13012.test.ts diff --git a/bin/cli/commands/mcp.mjs b/bin/cli/commands/mcp.mjs index c4ce9fbda8..0e740c4ac2 100644 --- a/bin/cli/commands/mcp.mjs +++ b/bin/cli/commands/mcp.mjs @@ -9,6 +9,8 @@ function truncate(v, len = 60) { return s.length > len ? s.slice(0, len - 1) + "…" : s; } +const VALID_MCP_TRANSPORTS = ["stdio", "sse", "streamable-http"]; + const mcpToolSchema = [ { key: "name", header: "Tool", width: 36 }, { @@ -43,6 +45,25 @@ export function registerMcp(program) { if (exitCode !== 0) process.exit(exitCode); }); + mcp + .command("enable") + .description(t("mcp.enable.description")) + .option("--transport ", t("mcp.enable.transport")) + .action(async (opts, cmd) => { + const globalOpts = cmd.parent.optsWithGlobals(); + const exitCode = await runMcpEnableCommand({ ...opts, output: globalOpts.output }); + if (exitCode !== 0) process.exit(exitCode); + }); + + mcp + .command("disable") + .description(t("mcp.disable.description")) + .action(async (opts, cmd) => { + const globalOpts = cmd.parent.optsWithGlobals(); + const exitCode = await runMcpDisableCommand({ ...opts, output: globalOpts.output }); + if (exitCode !== 0) process.exit(exitCode); + }); + // 5.1 — mcp call + mcp scopes mcp .command("call [argsJson]") @@ -61,10 +82,15 @@ export function registerMcp(program) { ? JSON.parse(argsPositional) : {}; - const exitCode = await runMcpCallCommand(tool, args, { - ...opts, - stream: opts.stream, - }, globalOpts); + const exitCode = await runMcpCallCommand( + tool, + args, + { + ...opts, + stream: opts.stream, + }, + globalOpts + ); if (exitCode !== 0) process.exit(exitCode); }); @@ -127,7 +153,9 @@ async function mcpJsonRpcCall(tool, args, { stream = false, globalOpts = {} } = if (!initRes.ok) { const text = await initRes.text().catch(() => ""); - process.stderr.write(`MCP initialize failed: HTTP ${initRes.status}${text ? ` — ${text}` : ""}\n`); + process.stderr.write( + `MCP initialize failed: HTTP ${initRes.status}${text ? ` — ${text}` : ""}\n` + ); return 1; } @@ -227,6 +255,7 @@ export async function runMcpStatusCommand(opts = {}) { }); if (!res.ok) { console.log(t("mcp.stopped")); + console.log(t("mcp.stoppedHint")); return 0; } @@ -240,6 +269,9 @@ export async function runMcpStatusCommand(opts = {}) { const transport = status.transport || "stdio"; const online = status.online ?? status.running; console.log(online ? t("mcp.running", { transport }) : t("mcp.stopped")); + if (!online && status.enabled === false) { + console.log(t("mcp.stoppedHint")); + } if (status.toolsCount !== undefined) console.log(` Tools: ${status.toolsCount}`); if (status.scopes?.length) { console.log(" Scopes:"); @@ -270,10 +302,76 @@ export async function runMcpRestartCommand(opts = {}) { console.log(t("mcp.restarted")); return 0; } - console.error(t("common.error", { message: `HTTP ${res.status}` })); + const body = await res.json().catch(() => null); + const message = body?.error || `HTTP ${res.status}`; + console.error(t("common.error", { message })); return 1; } catch (err) { console.error(t("common.error", { message: err instanceof Error ? err.message : String(err) })); return 1; } } + +export async function runMcpEnableCommand(opts = {}) { + const serverUp = await isServerUp(); + if (!serverUp) { + console.error(t("common.serverOffline")); + return 1; + } + + if (opts.transport && !VALID_MCP_TRANSPORTS.includes(opts.transport)) { + console.error( + t("common.error", { + message: `Invalid transport '${opts.transport}'. Valid: ${VALID_MCP_TRANSPORTS.join(", ")}`, + }) + ); + return 1; + } + + try { + const body = { mcpEnabled: true }; + if (opts.transport) body.mcpTransport = opts.transport; + + const res = await apiFetch("/api/settings", { + method: "PATCH", + body, + retry: false, + acceptNotOk: true, + }); + if (!res.ok) { + console.error(t("common.error", { message: `HTTP ${res.status}` })); + return 1; + } + console.log(t("mcp.enabled")); + return 0; + } catch (err) { + console.error(t("common.error", { message: err instanceof Error ? err.message : String(err) })); + return 1; + } +} + +export async function runMcpDisableCommand(opts = {}) { + const serverUp = await isServerUp(); + if (!serverUp) { + console.error(t("common.serverOffline")); + return 1; + } + + try { + const res = await apiFetch("/api/settings", { + method: "PATCH", + body: { mcpEnabled: false }, + retry: false, + acceptNotOk: true, + }); + if (!res.ok) { + console.error(t("common.error", { message: `HTTP ${res.status}` })); + return 1; + } + console.log(t("mcp.disabled")); + return 0; + } catch (err) { + console.error(t("common.error", { message: err instanceof Error ? err.message : String(err) })); + return 1; + } +} diff --git a/bin/cli/locales/en.json b/bin/cli/locales/en.json index 3ed2f2dbcf..4f5dc467e0 100644 --- a/bin/cli/locales/en.json +++ b/bin/cli/locales/en.json @@ -347,6 +347,16 @@ "running": "MCP server running ({transport})", "stopped": "MCP server stopped.", "restarted": "MCP server restarted.", + "stoppedHint": "Run `omniroute mcp enable` to turn it on.", + "enabled": "MCP server enabled.", + "disabled": "MCP server disabled.", + "enable": { + "description": "Enable the MCP server", + "transport": "Transport to use: stdio|sse|streamable-http" + }, + "disable": { + "description": "Disable the MCP server" + }, "call": { "description": "Invoke an MCP tool directly", "args": "JSON arguments object (inline)", diff --git a/bin/cli/locales/zh-CN.json b/bin/cli/locales/zh-CN.json index 31be9d4c16..31b9255745 100644 --- a/bin/cli/locales/zh-CN.json +++ b/bin/cli/locales/zh-CN.json @@ -345,6 +345,16 @@ "running": "MCP 服务器正在运行({transport})", "stopped": "MCP 服务器已停止。", "restarted": "MCP 服务器已重启。", + "stoppedHint": "运行 `omniroute mcp enable` 以启用它。", + "enabled": "MCP 服务器已启用。", + "disabled": "MCP 服务器已禁用。", + "enable": { + "description": "启用 MCP 服务器", + "transport": "要使用的传输方式:stdio|sse|streamable-http" + }, + "disable": { + "description": "禁用 MCP 服务器" + }, "call": { "description": "直接调用 MCP 工具", "args": "JSON 参数对象(内联)", diff --git a/bin/cli/locales/zh-TW.json b/bin/cli/locales/zh-TW.json index fa7ca866b8..c5c1a3dda5 100644 --- a/bin/cli/locales/zh-TW.json +++ b/bin/cli/locales/zh-TW.json @@ -345,6 +345,16 @@ "running": "MCP 伺服器正在執行({transport})", "stopped": "MCP 伺服器已停止。", "restarted": "MCP 伺服器已重啟。", + "stoppedHint": "執行 `omniroute mcp enable` 以啟用它。", + "enabled": "MCP 伺服器已啟用。", + "disabled": "MCP 伺服器已停用。", + "enable": { + "description": "啟用 MCP 伺服器", + "transport": "要使用的傳輸方式:stdio|sse|streamable-http" + }, + "disable": { + "description": "停用 MCP 伺服器" + }, "call": { "description": "直接呼叫 MCP 工具", "args": "JSON 引數物件(內聯)", diff --git a/changelog.d/fixes/13012-cli-mcp-restart-and-enable.md b/changelog.d/fixes/13012-cli-mcp-restart-and-enable.md new file mode 100644 index 0000000000..eb5c5f4a90 --- /dev/null +++ b/changelog.d/fixes/13012-cli-mcp-restart-and-enable.md @@ -0,0 +1 @@ +- **fix(cli):** `omniroute mcp restart` no longer 404s — the missing `POST /api/mcp/restart` route now exists — and new `omniroute mcp enable`/`mcp disable [--transport]` subcommands give the CLI a way to turn the MCP server on without the dashboard ([#13012](https://github.com/diegosouzapw/OmniRoute/issues/13012)) — thanks @ricardusx diff --git a/docs/frameworks/MCP-SERVER.md b/docs/frameworks/MCP-SERVER.md index b0413dfa26..b823e6add4 100644 --- a/docs/frameworks/MCP-SERVER.md +++ b/docs/frameworks/MCP-SERVER.md @@ -25,6 +25,23 @@ Or via the open-sse transport: omniroute --dev # MCP auto-starts on /mcp endpoint ``` +The HTTP transports (`sse` / `streamable-http`, served in-process by the dashboard server) are +off by default and were previously toggleable only from the `/dashboard/mcp` page. As of v3.8.51 +the CLI has parity: + +```bash +omniroute mcp status # enabled/online, transport, tool count +omniroute mcp enable [--transport stdio|sse|streamable-http] +omniroute mcp disable +omniroute mcp restart # resets active sse/streamable-http sessions +``` + +`mcp enable`/`mcp disable` PATCH the same `mcpEnabled` (and optionally `mcpTransport`) setting +the dashboard toggles via `/api/settings`. `mcp restart` calls `POST /api/mcp/restart`: it tears +down active `sse`/`streamable-http` sessions so the next request re-initializes cleanly, returns +`409` if MCP is disabled, and `501` for the `stdio` transport (stdio clients own their own +subprocess — there is no in-process handle to restart). + ## Transports The MCP server exposes three transports, all backed by the same `createMcpServer()` factory: diff --git a/src/app/api/mcp/restart/route.ts b/src/app/api/mcp/restart/route.ts new file mode 100644 index 0000000000..b675f04752 --- /dev/null +++ b/src/app/api/mcp/restart/route.ts @@ -0,0 +1,53 @@ +import { NextResponse } from "next/server"; +import { getMcpHttpStatus, shutdownMcpHttp } from "@omniroute/open-sse/mcp-server/httpTransport"; +import { getCachedSettings } from "@/lib/db/settings"; +import { requireManagementAuth } from "@/lib/api/requireManagementAuth"; + +/** + * POST /api/mcp/restart — resets the in-process MCP HTTP/SSE transport so the + * next MCP request re-initializes cleanly (mirrors the lazy-start design + * documented at the top of open-sse/mcp-server/httpTransport.ts). There is no + * external MCP process to restart the way `/api/restart` self-restarts the + * whole server — this only tears down active SSE/Streamable HTTP sessions. + * + * Fixes #13012: the CLI's `omniroute mcp restart` POSTs here but the route + * never existed, so every call 404d. + */ +export async function POST(request: Request) { + const authError = await requireManagementAuth(request, { acceptMcpConnectScope: true }); + if (authError) return authError; + + const settings = await getCachedSettings(); + const mcpEnabled = !!settings.mcpEnabled; + const mcpTransport = (settings.mcpTransport as string) || "stdio"; + + if (!mcpEnabled) { + return NextResponse.json( + { + error: "MCP is disabled; enable it first (`omniroute mcp enable`).", + }, + { status: 409 } + ); + } + + if (mcpTransport === "stdio") { + return NextResponse.json( + { + error: + "MCP restart is not supported for the stdio transport — stdio clients spawn their " + + "own subprocess with no in-process handle to restart. Switch to sse/streamable-http " + + "(`omniroute mcp enable --transport sse`) or restart the client instead.", + }, + { status: 501 } + ); + } + + shutdownMcpHttp(); + + return NextResponse.json({ + status: "restarted", + enabled: mcpEnabled, + transport: mcpTransport, + httpTransport: getMcpHttpStatus(), + }); +} diff --git a/tests/unit/cli-mcp-enable-disable-13012.test.ts b/tests/unit/cli-mcp-enable-disable-13012.test.ts new file mode 100644 index 0000000000..0f43e483de --- /dev/null +++ b/tests/unit/cli-mcp-enable-disable-13012.test.ts @@ -0,0 +1,95 @@ +// Regression for GitHub issue #13012 (Bug 2): there was no CLI/env path to +// flip the mcpEnabled setting on — it was dashboard-only. Pins the new +// `omniroute mcp enable`/`mcp disable` PATCH /api/settings body shape. +import test from "node:test"; +import assert from "node:assert/strict"; + +const ORIGINAL_FETCH = globalThis.fetch; + +function makeResp(data: unknown, status = 200) { + return { + ok: status < 400, + status, + json: () => Promise.resolve(data), + text: () => Promise.resolve(JSON.stringify(data)), + headers: new Headers({ "content-type": "application/json" }), + }; +} + +type Call = { url: string; method: string; body: unknown }; + +function mockFetch(calls: Call[]) { + globalThis.fetch = (async (url: string, opts: Record = {}) => { + const u = String(url); + const method = String(opts.method || "GET").toUpperCase(); + const body = opts.body ? JSON.parse(String(opts.body)) : null; + calls.push({ url: u, method, body }); + if (u.includes("/api/health")) return makeResp({ status: "ok" }) as unknown as Response; + if (u.includes("/api/settings")) return makeResp({ ok: true }) as unknown as Response; + return makeResp({ error: "unexpected call" }, 404) as unknown as Response; + }) as typeof fetch; +} + +async function withMockedFetch(fn: (calls: Call[]) => Promise) { + const calls: Call[] = []; + mockFetch(calls); + const originalLog = console.log; + const originalError = console.error; + console.log = () => {}; + console.error = () => {}; + try { + await fn(calls); + } finally { + console.log = originalLog; + console.error = originalError; + globalThis.fetch = ORIGINAL_FETCH; + } +} + +test("mcp enable PATCHes /api/settings with mcpEnabled: true", async () => { + await withMockedFetch(async (calls) => { + const { runMcpEnableCommand } = await import("../../bin/cli/commands/mcp.mjs"); + const result = await runMcpEnableCommand({}); + assert.equal(result, 0); + + const patchCall = calls.find((c) => c.url.includes("/api/settings") && c.method === "PATCH"); + assert.ok(patchCall, "expected a PATCH /api/settings call"); + assert.deepEqual(patchCall!.body, { mcpEnabled: true }); + }); +}); + +test("mcp enable --transport sse also sets mcpTransport in the same PATCH", async () => { + await withMockedFetch(async (calls) => { + const { runMcpEnableCommand } = await import("../../bin/cli/commands/mcp.mjs"); + const result = await runMcpEnableCommand({ transport: "sse" }); + assert.equal(result, 0); + + const patchCall = calls.find((c) => c.url.includes("/api/settings") && c.method === "PATCH"); + assert.ok(patchCall); + assert.deepEqual(patchCall!.body, { mcpEnabled: true, mcpTransport: "sse" }); + }); +}); + +test("mcp enable rejects an invalid --transport value without calling the API", async () => { + await withMockedFetch(async (calls) => { + const { runMcpEnableCommand } = await import("../../bin/cli/commands/mcp.mjs"); + const result = await runMcpEnableCommand({ transport: "bogus" }); + assert.equal(result, 1); + assert.ok( + !calls.some((c) => c.url.includes("/api/settings")), + "invalid transport must not reach the settings API" + ); + }); +}); + +test("mcp disable PATCHes /api/settings with mcpEnabled: false", async () => { + await withMockedFetch(async (calls) => { + const { runMcpDisableCommand } = await import("../../bin/cli/commands/mcp.mjs"); + const result = await runMcpDisableCommand({}); + assert.equal(result, 0); + + const patchCall = calls.find((c) => c.url.includes("/api/settings") && c.method === "PATCH"); + assert.ok(patchCall, "expected a PATCH /api/settings call"); + assert.deepEqual(patchCall!.body, { mcpEnabled: false }); + }); +}); diff --git a/tests/unit/mcp-restart-route-13012.test.ts b/tests/unit/mcp-restart-route-13012.test.ts new file mode 100644 index 0000000000..956cd5f0c3 --- /dev/null +++ b/tests/unit/mcp-restart-route-13012.test.ts @@ -0,0 +1,77 @@ +// Regression for GitHub issue #13012 (Bug 2): `omniroute mcp restart` POSTs to +// /api/mcp/restart, but that route never existed — every call 404d. This test +// boots the route handler directly and pins its behavior across the three +// states the CLI can hit: disabled, enabled+stdio, enabled+http-family. +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omr-mcp-restart-route-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts"); +const settingsDb = await import("../../src/lib/db/settings.ts"); +const { POST } = await import("../../src/app/api/mcp/restart/route.ts"); +const { shutdownMcpHttp } = await import("../../open-sse/mcp-server/httpTransport.ts"); +const { isLocalOnlyPath } = await import("../../src/server/authz/routeGuard.ts"); + +// Hard Rule #15: /api/mcp/ must stay LOCAL_ONLY so loopback enforcement runs +// unconditionally before any auth check — a leaked JWT over a tunnel must not +// reach a route that can tear down/spin up MCP transport sessions. +test("issue #13012: POST /api/mcp/restart is classified LOCAL_ONLY", () => { + assert.equal(isLocalOnlyPath("/api/mcp/restart"), true); +}); + +function reset() { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.beforeEach(async () => { + reset(); + // No JWT_SECRET / requireLogin=false ⇒ auth-disabled deployment (pre-existing + // open-door contract also exercised by tests/unit/mcp-route-scope-carveout.test.ts). + await settingsDb.updateSettings({ requireLogin: false }); +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +function restartRequest(): Request { + return new Request("http://localhost:20128/api/mcp/restart", { method: "POST" }); +} + +test("issue #13012: POST /api/mcp/restart exists and returns 409 when MCP is disabled", async () => { + await settingsDb.updateSettings({ mcpEnabled: false }); + const res = await POST(restartRequest()); + assert.equal(res.status, 409); + const body = (await res.json()) as { error?: string }; + assert.match(body.error ?? "", /disabled/i); +}); + +test("POST /api/mcp/restart returns 501 for the stdio transport (no in-process handle)", async () => { + await settingsDb.updateSettings({ mcpEnabled: true, mcpTransport: "stdio" }); + const res = await POST(restartRequest()); + assert.equal(res.status, 501); + const body = (await res.json()) as { error?: string }; + assert.match(body.error ?? "", /stdio/i); +}); + +test("POST /api/mcp/restart tears down HTTP sessions and returns 200 for sse/streamable-http", async () => { + await settingsDb.updateSettings({ mcpEnabled: true, mcpTransport: "sse" }); + const res = await POST(restartRequest()); + assert.equal(res.status, 200); + const body = (await res.json()) as { status?: string; enabled?: boolean; transport?: string }; + assert.equal(body.status, "restarted"); + assert.equal(body.enabled, true); + assert.equal(body.transport, "sse"); +}); + +test("shutdownMcpHttp is exported and callable (route depends on this contract)", () => { + assert.equal(typeof shutdownMcpHttp, "function"); +});