From 354a594bac113536461c374cbaa223ef96007604 Mon Sep 17 00:00:00 2001 From: Filip Klosowski Date: Fri, 24 Jul 2026 09:18:36 +0200 Subject: [PATCH 1/2] Fix for MCP server crashes --- lib/logger.js | 17 +++++ lib/settings.js | 8 +- mcp-min/__tests__/auth.env-resolve.test.js | 75 +++++++++++++++++++ mcp-min/__tests__/logger.server-mode.test.js | 79 ++++++++++++++++++++ mcp-min/auth.js | 2 +- mcp-min/index.js | 6 ++ 6 files changed, 185 insertions(+), 2 deletions(-) create mode 100644 mcp-min/__tests__/auth.env-resolve.test.js create mode 100644 mcp-min/__tests__/logger.server-mode.test.js diff --git a/lib/logger.js b/lib/logger.js index 87f89f04..fe2548c7 100644 --- a/lib/logger.js +++ b/lib/logger.js @@ -11,6 +11,17 @@ const isSimple = !!process.env.NO_COLOR || !!process.env.CI; let instance = null; let instancePromise = null; +// Server mode: when a long-lived host (the MCP server) loads this CLI code +// in-process, a fatal logger.Error must NOT process.exit — that would tear +// down the whole server and every tool it serves. In server mode Error throws +// instead, so the host's per-request try/catch turns it into a caught error. +// Sourced from an env var so it survives across duplicate module instances. +const isServerMode = () => process.env.POS_CLI_SERVER_MODE === '1'; +const setServerMode = (on) => { + if (on) process.env.POS_CLI_SERVER_MODE = '1'; + else delete process.env.POS_CLI_SERVER_MODE; +}; + const loadLogger = async () => { if (instance) return instance; if (instancePromise) return instancePromise; @@ -74,6 +85,11 @@ const logger = { await callInstance('Error', [msg]).catch(() => {}); if (options.exit) { + // In server mode, throw so the host can catch it; exiting would kill + // the whole server. Standalone CLI keeps the process.exit contract. + if (isServerMode()) { + throw new Error(msg); + } process.exit(1); } @@ -97,3 +113,4 @@ const logger = { }; export default logger; +export { setServerMode, isServerMode }; diff --git a/lib/settings.js b/lib/settings.js index ef9534ab..cf49e078 100644 --- a/lib/settings.js +++ b/lib/settings.js @@ -50,10 +50,16 @@ const loadSettingsFileForModule = module => { return base; }; -const fetchSettings = async (environment) => { +const fetchSettings = async (environment, { exit = true } = {}) => { const settings = settingsFromEnv() || settingsFromDotPos(environment); if (settings) return settings; + // Long-lived callers (e.g. the MCP server) pass { exit: false } so an + // unresolved environment is a recoverable miss they can turn into a caught + // error — never a process.exit that would tear down the whole server. CLI + // callers keep the default exit:true for the usual "print message and die". + if (!exit) return null; + if (environment) { await logger.Error(`No settings for ${environment} environment, please see pos-cli env add`); } else { diff --git a/mcp-min/__tests__/auth.env-resolve.test.js b/mcp-min/__tests__/auth.env-resolve.test.js new file mode 100644 index 00000000..99e96a94 --- /dev/null +++ b/mcp-min/__tests__/auth.env-resolve.test.js @@ -0,0 +1,75 @@ +import fs from 'fs'; +import path from 'path'; +import { vi, describe, test, expect, beforeAll, afterAll, beforeEach } from 'vitest'; + +import { fetchSettings } from '../../lib/settings.js'; +import { resolveAuth } from '../auth.js'; + +// Regression guard for the "env not found kills the MCP server" crash. +// +// The server resolves auth via resolveAuth -> fetchSettings. Historically +// fetchSettings did process.exit(1) on an unknown environment, which killed +// the whole in-process server before auth.js could throw its catchable error. +// The fix: fetchSettings(env, { exit:false }) returns null so resolveAuth can +// throw a normal Error the per-request handler turns into an MCP error. +const CONFIG_FILE = path.resolve(`.pos.test-auth-resolve`); + +describe('env resolution never exits the process', () => { + beforeAll(() => { + fs.writeFileSync( + CONFIG_FILE, + JSON.stringify({ staging: { url: 'https://staging.example.com', email: 'e@x', token: 't' } }, null, 2) + ); + process.env.CONFIG_FILE_PATH = CONFIG_FILE; + }); + + afterAll(() => { + if (fs.existsSync(CONFIG_FILE)) fs.unlinkSync(CONFIG_FILE); + delete process.env.CONFIG_FILE_PATH; + }); + + beforeEach(() => { + // MPKIT_* would short-circuit settingsFromEnv(); keep resolution on .pos. + delete process.env.MPKIT_URL; + delete process.env.MPKIT_EMAIL; + delete process.env.MPKIT_TOKEN; + }); + + test('fetchSettings(unknown, {exit:false}) returns null instead of exiting', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit must not be called with {exit:false}'); + }); + + const found = await fetchSettings('does-not-exist', { exit: false }); + expect(found).toBeNull(); + expect(exitSpy).not.toHaveBeenCalled(); + + exitSpy.mockRestore(); + }); + + test('fetchSettings(known, {exit:false}) returns the settings', async () => { + const found = await fetchSettings('staging', { exit: false }); + expect(found).toMatchObject({ url: 'https://staging.example.com' }); + }); + + test('resolveAuth throws a catchable error for an unknown env (no exit)', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit must not be called from resolveAuth'); + }); + + await expect(resolveAuth({ env: 'ps' })).rejects.toThrow(/Environment 'ps' not found/); + expect(exitSpy).not.toHaveBeenCalled(); + + exitSpy.mockRestore(); + }); + + test('resolveAuth resolves a known env from .pos', async () => { + const auth = await resolveAuth({ env: 'staging' }); + expect(auth).toMatchObject({ url: 'https://staging.example.com', source: '.pos(staging)' }); + }); + + test('explicit url/email/token params bypass .pos resolution', async () => { + const auth = await resolveAuth({ url: 'https://direct', email: 'e@x', token: 'tok' }); + expect(auth).toMatchObject({ url: 'https://direct', source: 'params' }); + }); +}); diff --git a/mcp-min/__tests__/logger.server-mode.test.js b/mcp-min/__tests__/logger.server-mode.test.js new file mode 100644 index 00000000..0746e62d --- /dev/null +++ b/mcp-min/__tests__/logger.server-mode.test.js @@ -0,0 +1,79 @@ +import { vi, describe, test, expect, beforeAll, afterEach } from 'vitest'; + +// The global test setup (test/vitest-setup.js) mocks #lib/logger.js to silence +// output. This file must exercise the REAL logger to verify server-mode +// behavior, so pull the actual implementation via importActual. +let logger; +let setServerMode; +let isServerMode; + +beforeAll(async () => { + const actual = await vi.importActual('../../lib/logger.js'); + logger = actual.default; + setServerMode = actual.setServerMode; + isServerMode = actual.isServerMode; +}); + +// Regression guard for the MCP-server crash: logger.Error is the single +// process-exit choke point in the CLI. Loaded in-process by the long-lived +// MCP server, a process.exit(1) there tears down the whole server and every +// tool it serves. In server mode Error must THROW (catchable per-request) +// instead; standalone CLI must keep the process.exit(1) contract. +describe('logger server-mode hardening', () => { + afterEach(() => { + setServerMode(false); + vi.restoreAllMocks(); + }); + + test('setServerMode toggles the flag', () => { + setServerMode(false); + expect(isServerMode()).toBe(false); + setServerMode(true); + expect(isServerMode()).toBe(true); + setServerMode(false); + expect(isServerMode()).toBe(false); + }); + + test('server mode: Error({exit:true}) throws and does NOT call process.exit', async () => { + setServerMode(true); + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit must not be called in server mode'); + }); + + await expect( + logger.Error('boom in server mode', { notify: false }) + ).rejects.toThrow(/boom in server mode/); + + expect(exitSpy).not.toHaveBeenCalled(); + }); + + test('CLI mode: Error({exit:true}) still calls process.exit(1)', async () => { + setServerMode(false); + // Mock exit to a no-op so vitest itself survives; assert it was invoked. + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => {}); + + const ret = await logger.Error('cli fatal', { notify: false }); + + expect(exitSpy).toHaveBeenCalledWith(1); + // After the (mocked) exit, control falls through to `return false`. + expect(ret).toBe(false); + }); + + test('exit:false: Error never throws and never exits, in either mode', async () => { + const exitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { + throw new Error('process.exit must not be called for exit:false'); + }); + + setServerMode(true); + await expect( + logger.Error('soft error', { exit: false, notify: false }) + ).resolves.toBe(false); + + setServerMode(false); + await expect( + logger.Error('soft error', { exit: false, notify: false }) + ).resolves.toBe(false); + + expect(exitSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/mcp-min/auth.js b/mcp-min/auth.js index 138b0163..e1b4d47f 100644 --- a/mcp-min/auth.js +++ b/mcp-min/auth.js @@ -37,7 +37,7 @@ export async function resolveAuth(params, ctx = {}) { // Priority 2: Named .pos environment. When an env name is given we resolve it // directly without falling back to MPKIT_* — the caller is being explicit. if (params?.env) { - const found = await settingsModule.fetchSettings(params.env); + const found = await settingsModule.fetchSettings(params.env, { exit: false }); if (found) return { ...found, source: `.pos(${params.env})` }; throw new Error(`Environment '${params.env}' not found in .pos config`); } diff --git a/mcp-min/index.js b/mcp-min/index.js index 1e52a5b3..e67b9829 100644 --- a/mcp-min/index.js +++ b/mcp-min/index.js @@ -1,6 +1,12 @@ import startStdio from './stdio-server.js'; import startHttp from './http-server.js'; import log from './log.js'; +import { setServerMode } from '../lib/logger.js'; + +// This host loads CLI code in-process; a fatal logger.Error must throw (caught +// per-request), never process.exit and take down all tools. Enable before any +// tool can run. +setServerMode(true); const PORT = process.env.MCP_MIN_PORT || 5910; From 0a8db17a4f3bfcb878e7c8013d5cafd31210f4b0 Mon Sep 17 00:00:00 2001 From: Filip Klosowski Date: Fri, 24 Jul 2026 10:22:48 +0200 Subject: [PATCH 2/2] MCP server exit crash fixes --- CHANGELOG.md | 8 +++++++- package-lock.json | 4 ++-- package.json | 2 +- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ef336e2..87dd93bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,12 @@ # Changelog -## 6.2.3 (2026-07-23) +## 6.2.4 (2026-07-24) + +### Fixes + +* `pos-cli-mcp` no longer crashes the entire server when a single tool hits a fatal error. The MCP server runs all tools in one process, so a fatal `logger.Error` — which calls `process.exit(1)` — used to tear down the whole server and every tool it exposed, forcing a reconnect. Under the MCP server, fatal errors now throw instead of exiting, so the per-request handler returns a clean error and the server stays up. Standalone CLI behavior is unchanged (it still exits on fatal errors). The most common trigger — a tool called with an unresolvable environment — now returns a caught error instead of killing the server. + + ### Fixes diff --git a/package-lock.json b/package-lock.json index ae687557..63c84fdd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@platformos/pos-cli", - "version": "6.2.3", + "version": "6.2.4", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@platformos/pos-cli", - "version": "6.2.3", + "version": "6.2.4", "bundleDependencies": [ "commander", "degit", diff --git a/package.json b/package.json index a37770c8..55672812 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@platformos/pos-cli", - "version": "6.2.3", + "version": "6.2.4", "description": "Manage your platformOS application", "type": "module", "imports": {