diff --git a/package.json b/package.json index 5212da6..3a44a2d 100644 --- a/package.json +++ b/package.json @@ -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": { diff --git a/src/ide-bridge.ts b/src/ide-bridge.ts index 2645eb2..57cbf78 100644 --- a/src/ide-bridge.ts +++ b/src/ide-bridge.ts @@ -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 ""`, 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); diff --git a/src/server.ts b/src/server.ts index 06faeba..8106a8f 100644 --- a/src/server.ts +++ b/src/server.ts @@ -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 { // 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 { 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 { 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 { +): Promise { + // 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; }