fix(ide-bridge): reap CodesysMCPBridge.exe so it never orphans
registerIdeBridgeTools() spawned CodesysMCPBridge.exe but the SIGINT/SIGTERM shutdown handler never closed the client, so every orchestrator exit left the shim running. They piled up across sessions (7 observed live with no IDE open).
- ide-bridge: add findOrphanedBridgePids()/killOrphanedBridges() using dead-parent detection (only reaps shims whose parent process is gone, so live sessions are untouched); harden close() with a taskkill /F fallback; expose .pid getter.
- server: sweep orphaned shims at startup, track the bridge client and close() it on shutdown, and add a process.on('exit') taskkill safety net for non-signal exits (stdin EOF / fatal).
Verified end-to-end against compiled dist: real orphan detected+reaped, live-parent shim left alone. Build clean, 156 tests pass. Bump 0.10.1 -> 0.10.2.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Karstein Phobic Nyvold Kvistad
parent
d801038dda
commit
fb8b5b0c67
3 changed files with 135 additions and 8 deletions
|
|
@ -1,6 +1,6 @@
|
|||
{
|
||||
"name": "codesys-mcp-sp21-plus",
|
||||
"version": "0.10.1",
|
||||
"version": "0.10.2",
|
||||
"description": "Codesys-MCP-SP21+ -- fork of luke-harriman/Codesys-MCP carrying CODESYS V3.5 SP22 Patch 1 fixes (and forward-compat with later SPs): script-engine API drift, online/runtime tool auto-login, dual-SHA release classifier, set_pou_code omitted-decl wipe fix, add_library managed-overload, etc. MCP server for CODESYS with persistent UI instance and file-based IPC.",
|
||||
"main": "dist/server.js",
|
||||
"bin": {
|
||||
|
|
|
|||
|
|
@ -15,12 +15,89 @@
|
|||
* the home of every online/runtime/release tool the bridge doesn't ship.
|
||||
*/
|
||||
|
||||
import { spawn, ChildProcessWithoutNullStreams } from 'child_process';
|
||||
import { spawn, execSync, ChildProcessWithoutNullStreams } from 'child_process';
|
||||
import * as path from 'path';
|
||||
import * as fs from 'fs';
|
||||
import { z } from 'zod';
|
||||
import { serverLog } from './logger';
|
||||
|
||||
/** Image name of the CODESYS-shipped bridge shim, used by the orphan sweep. */
|
||||
const BRIDGE_IMAGE_NAME = 'CodesysMCPBridge.exe';
|
||||
|
||||
/**
|
||||
* Find CodesysMCPBridge.exe processes that have been orphaned — i.e. whose
|
||||
* parent process is no longer alive. Such a bridge can never be re-attached
|
||||
* to (the stdio pipe that drove it died with its parent), so it is pure
|
||||
* garbage and safe to kill.
|
||||
*
|
||||
* Why dead-parent rather than "all bridges": a single sweep must not touch a
|
||||
* bridge that belongs to a *live* MCP session — both the `codesys-ide` direct
|
||||
* server (spawned by the MCP client) and a concurrently-running orchestrator
|
||||
* keep a living parent, so they are correctly preserved. Only the leftovers
|
||||
* from sessions that already exited are reaped.
|
||||
*
|
||||
* Returns an empty list on non-Windows or if the probe fails — we treat that
|
||||
* as "can't tell" rather than risk killing something we shouldn't.
|
||||
*/
|
||||
export function findOrphanedBridgePids(): number[] {
|
||||
if (process.platform !== 'win32') return [];
|
||||
try {
|
||||
// One PowerShell pass: build a set of all live PIDs, then keep the bridge
|
||||
// processes whose ParentProcessId is absent from it.
|
||||
//
|
||||
// Note: only SINGLE quotes are used inside the command. The whole script is
|
||||
// passed via `-Command "<here>"`, so an embedded double quote (e.g. a CIM
|
||||
// `-Filter "Name='...'"`) would prematurely close that argument and the
|
||||
// command would silently fail — hence Where-Object on $_.Name instead.
|
||||
// Both the live-PID keys and the ParentProcessId lookup are cast to [int]
|
||||
// so an int key never misses a UInt32 ParentProcessId.
|
||||
// ConvertTo-Json emits a bare value for a single match, an array for many.
|
||||
const ps =
|
||||
`$alive=@{}; Get-Process -ErrorAction SilentlyContinue | ForEach-Object { $alive[[int]$_.Id]=$true }; ` +
|
||||
`Get-CimInstance Win32_Process -ErrorAction SilentlyContinue ` +
|
||||
`| Where-Object { $_.Name -eq '${BRIDGE_IMAGE_NAME}' -and -not $alive[[int]$_.ParentProcessId] } ` +
|
||||
`| Select-Object -ExpandProperty ProcessId ` +
|
||||
`| ConvertTo-Json -Compress`;
|
||||
const out = execSync(
|
||||
`powershell -NoProfile -ExecutionPolicy Bypass -Command "${ps}"`,
|
||||
{ encoding: 'utf-8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 10_000 }
|
||||
);
|
||||
const trimmed = out.trim();
|
||||
if (!trimmed) return [];
|
||||
const parsed = JSON.parse(trimmed);
|
||||
const arr: unknown[] = Array.isArray(parsed) ? parsed : [parsed];
|
||||
return arr.filter((v): v is number => typeof v === 'number' && Number.isInteger(v));
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Kill any orphaned bridge shims (see {@link findOrphanedBridgePids}). Returns
|
||||
* the PIDs actually reaped. Called at server startup so each fresh orchestrator
|
||||
* cleans up the garbage left by sessions that exited without reaping their
|
||||
* bridge (e.g. a hard SIGKILL of a prior orchestrator, or a stale `codesys-ide`
|
||||
* direct-server bridge whose MCP client has since closed).
|
||||
*/
|
||||
export function killOrphanedBridges(): number[] {
|
||||
const pids = findOrphanedBridgePids();
|
||||
const killed: number[] = [];
|
||||
for (const pid of pids) {
|
||||
try {
|
||||
execSync(`taskkill /PID ${pid}`, { timeout: 5000, stdio: 'ignore' });
|
||||
killed.push(pid);
|
||||
} catch {
|
||||
try {
|
||||
execSync(`taskkill /F /PID ${pid}`, { timeout: 5000, stdio: 'ignore' });
|
||||
killed.push(pid);
|
||||
} catch {
|
||||
// ignore — a survivor will show up on the next sweep
|
||||
}
|
||||
}
|
||||
}
|
||||
return killed;
|
||||
}
|
||||
|
||||
/** Tool descriptor returned by the bridge's tools/list. */
|
||||
export interface BridgeTool {
|
||||
name: string;
|
||||
|
|
@ -59,6 +136,11 @@ export class IdeBridgeClient {
|
|||
|
||||
constructor(private readonly exePath: string) {}
|
||||
|
||||
/** PID of the spawned bridge shim, or null if not currently connected. */
|
||||
get pid(): number | null {
|
||||
return this.proc?.pid ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Default location of the bridge shim alongside CODESYS.exe. Returns null
|
||||
* if the bridge isn't installed (SP19/SP21, or SP22+ before the user has
|
||||
|
|
@ -138,12 +220,23 @@ export class IdeBridgeClient {
|
|||
close(): void {
|
||||
if (!this.connected) return;
|
||||
this.connected = false;
|
||||
const pid = this.proc?.pid;
|
||||
try {
|
||||
this.proc?.stdin.end();
|
||||
} catch {
|
||||
/* swallow */
|
||||
}
|
||||
this.proc?.kill();
|
||||
// Node's proc.kill() can silently no-op on Windows if the handle is
|
||||
// already stale, leaving an orphan. Force the kill via taskkill so the
|
||||
// shim never survives us.
|
||||
if (pid && process.platform === 'win32') {
|
||||
try {
|
||||
execSync(`taskkill /F /PID ${pid}`, { timeout: 5000, stdio: 'ignore' });
|
||||
} catch {
|
||||
/* already gone, or taskkill unavailable — best effort */
|
||||
}
|
||||
}
|
||||
this.proc = null;
|
||||
for (const [, p] of this.pending) {
|
||||
if (p.timer) clearTimeout(p.timer);
|
||||
|
|
|
|||
|
|
@ -22,7 +22,7 @@ import {
|
|||
formatRestartRuntimeResult,
|
||||
} from './ssh-restart-runtime';
|
||||
import { resolveMirrorRoot } from './mirror-paths';
|
||||
import { IdeBridgeClient, bridgeSchemaToZodShape } from './ide-bridge';
|
||||
import { IdeBridgeClient, bridgeSchemaToZodShape, killOrphanedBridges } from './ide-bridge';
|
||||
import { inspectProjectFile } from './inspect';
|
||||
import { parseProfileName } from './detect';
|
||||
import { decideOpenProjectPreflight } from './preflight';
|
||||
|
|
@ -3459,8 +3459,11 @@ export async function startMcpServer(config: ServerConfig): Promise<void> {
|
|||
// re-register each tool under an `ide_` prefix. The bridge's authoring
|
||||
// tools mutate the live project graph and the editor view picks the change
|
||||
// up immediately, which our IronPython watcher can't do.
|
||||
// Tracked so the shutdown handler can reap the bridge shim it spawned —
|
||||
// otherwise CodesysMCPBridge.exe orphans every time the orchestrator exits.
|
||||
let ideBridgeClient: IdeBridgeClient | null = null;
|
||||
if (config.ideBridge !== 'off') {
|
||||
await registerIdeBridgeTools(s, config.ideBridge, config.codesysPath);
|
||||
ideBridgeClient = await registerIdeBridgeTools(s, config.ideBridge, config.codesysPath);
|
||||
}
|
||||
|
||||
// ─── Connect ─────────────────────────────────────────────────────────
|
||||
|
|
@ -3546,6 +3549,14 @@ export async function startMcpServer(config: ServerConfig): Promise<void> {
|
|||
if (liveValuesPump) {
|
||||
liveValuesPump.stop();
|
||||
}
|
||||
if (ideBridgeClient) {
|
||||
// Reap the bridge shim we spawned so it doesn't orphan.
|
||||
try {
|
||||
ideBridgeClient.close();
|
||||
} catch {
|
||||
serverLog.warn('IDE bridge close failed during signal handler');
|
||||
}
|
||||
}
|
||||
if (launcher) {
|
||||
try {
|
||||
await launcher.shutdown();
|
||||
|
|
@ -3558,6 +3569,19 @@ export async function startMcpServer(config: ServerConfig): Promise<void> {
|
|||
|
||||
process.on('SIGINT', shutdown);
|
||||
process.on('SIGTERM', shutdown);
|
||||
// Last-ditch synchronous safety net: if we exit by a path that bypasses the
|
||||
// async shutdown() above (e.g. stdin EOF from the MCP client, or an
|
||||
// uncaught fatal), still force-kill the bridge shim so it can't orphan.
|
||||
process.on('exit', () => {
|
||||
const pid = ideBridgeClient?.pid;
|
||||
if (pid && process.platform === 'win32') {
|
||||
try {
|
||||
execSync(`taskkill /F /PID ${pid}`, { timeout: 5000, stdio: 'ignore' });
|
||||
} catch {
|
||||
/* best effort */
|
||||
}
|
||||
}
|
||||
});
|
||||
process.on('unhandledRejection', (reason) => {
|
||||
serverLog.error(`Unhandled rejection: ${reason}`);
|
||||
});
|
||||
|
|
@ -3573,7 +3597,16 @@ async function registerIdeBridgeTools(
|
|||
s: any,
|
||||
mode: 'on' | 'auto',
|
||||
codesysPath: string
|
||||
): Promise<void> {
|
||||
): Promise<IdeBridgeClient | null> {
|
||||
// Before spawning our own bridge, reap any bridge shims left orphaned by a
|
||||
// prior session that exited without reaping them (hard kill, or a stale
|
||||
// `codesys-ide` direct-server bridge whose MCP client has closed). Only
|
||||
// shims with a dead parent are touched, so live sessions are never disturbed.
|
||||
const reaped = killOrphanedBridges();
|
||||
if (reaped.length > 0) {
|
||||
serverLog.info(`Reaped ${reaped.length} orphaned bridge shim(s) at startup (PIDs: ${reaped.join(', ')}).`);
|
||||
}
|
||||
|
||||
const exe = IdeBridgeClient.defaultExePath(codesysPath);
|
||||
if (!exe) {
|
||||
if (mode === 'on') {
|
||||
|
|
@ -3583,7 +3616,7 @@ async function registerIdeBridgeTools(
|
|||
);
|
||||
}
|
||||
serverLog.info('IDE bridge shim not present on this CODESYS install; skipping (auto).');
|
||||
return;
|
||||
return null;
|
||||
}
|
||||
const client = new IdeBridgeClient(exe);
|
||||
try {
|
||||
|
|
@ -3596,7 +3629,7 @@ async function registerIdeBridgeTools(
|
|||
}
|
||||
serverLog.info(`IDE bridge not attached (auto): ${msg}`);
|
||||
client.close();
|
||||
return;
|
||||
return null;
|
||||
}
|
||||
let tools;
|
||||
try {
|
||||
|
|
@ -3604,7 +3637,7 @@ async function registerIdeBridgeTools(
|
|||
} catch (err) {
|
||||
serverLog.warn(`IDE bridge listTools failed: ${err instanceof Error ? err.message : err}`);
|
||||
client.close();
|
||||
return;
|
||||
return null;
|
||||
}
|
||||
serverLog.info(`IDE bridge attached. Registering ${tools.length} passthrough tool(s) with 'ide_' prefix.`);
|
||||
for (const tool of tools) {
|
||||
|
|
@ -3626,4 +3659,5 @@ async function registerIdeBridgeTools(
|
|||
}
|
||||
});
|
||||
}
|
||||
return client;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue