fix(js-debug): tag vsDebugServer argv and reap stranded instances (#431)

packages/adapter-javascript/src/javascript-debug-adapter.ts

@@ -25,7 +25,9 @@ import {
   type LanguageSpecificAttachConfig,
   type FeatureRequirement,
   type AdapterCapabilities,
-  type AdapterLaunchBarrier
+  type AdapterLaunchBarrier,
+  OWNER_PID_ARG_PREFIX,
+  SESSION_ID_ARG_PREFIX
 } from '@debugmcp/shared';
 import { DebugLanguage } from '@debugmcp/shared';
 import type { AdapterDependencies } from '@debugmcp/shared';
@@ -280,7 +282,22 @@ export class JavascriptDebugAdapter extends EventEmitter implements IDebugAdapte
       typeof config?.adapterHost === 'string' && config.adapterHost.trim().length > 0
         ? config.adapterHost
         : '127.0.0.1';
-    const args = [adapterPath, String(port), host];
+    // Reaper markers (issue #431): vsDebugServer.cjs reads only argv[2] (port)
+    // and argv[3] (host) and ignores trailing tokens, so these tags are inert
+    // at runtime — but they let the startup janitor recognize an instance
+    // stranded by a hard-killed proxy worker (spawned detached+unref'd, it
+    // survives every tree-kill path) and reap it by owner pid. Constraints:
+    // tokens must stay whitespace-free (the win32 process scan splits
+    // CommandLine on whitespace) and must never contain `--help` (vsDebugServer
+    // prints usage and exits if --help appears anywhere in argv).
+    const ownerPid = Number(process.env.MCP_DEBUGGER_MAIN_PID) || process.pid;
+    const args = [
+      adapterPath,
+      String(port),
+      host,
+      `${OWNER_PID_ARG_PREFIX}${ownerPid}`,
+      `${SESSION_ID_ARG_PREFIX}${config.sessionId}`
+    ];
 
     // Environment: clone from process.env (string values only), safely ensure NODE_OPTIONS memory flag
     const env: Record<string, string> = {};

packages/adapter-javascript/tests/unit/javascript-debug-adapter.command.test.ts

@@ -73,6 +73,41 @@ describe('JavascriptDebugAdapter.buildAdapterCommand (stdio)', () => {
     expect(path.isAbsolute(adapterPath as string)).toBe(true);
   });
 
+  it('tags the adapter argv with owner-pid and session-id reaper markers (issue #431)', () => {
+    const adapter = new JavascriptDebugAdapter(deps);
+
+    vi.stubEnv('MCP_DEBUGGER_MAIN_PID', '4242');
+    const cmd = adapter.buildAdapterCommand(defaultConfig);
+
+    // vsDebugServer.cjs reads only argv[2] (port) and argv[3] (host); trailing
+    // tokens are inert, which is what makes these markers safe to append.
+    expect(cmd.args[3]).toBe('--mcp-owner-pid=4242');
+    expect(cmd.args[4]).toBe('--mcp-session-id=test-session');
+  });
+
+  it('falls back to process.pid for the owner marker when MCP_DEBUGGER_MAIN_PID is unset', () => {
+    const adapter = new JavascriptDebugAdapter(deps);
+
+    vi.stubEnv('MCP_DEBUGGER_MAIN_PID', undefined);
+    const cmd = adapter.buildAdapterCommand(defaultConfig);
+
+    expect(cmd.args[3]).toBe(`--mcp-owner-pid=${process.pid}`);
+  });
+
+  it('marker tokens contain no whitespace and never the string --help', () => {
+    // Whitespace would fragment under the win32 CommandLine split; --help
+    // anywhere in argv makes vsDebugServer print usage instead of serving.
+    const adapter = new JavascriptDebugAdapter(deps);
+
+    const cmd = adapter.buildAdapterCommand(defaultConfig);
+
+    for (const token of cmd.args.slice(3)) {
+      expect(token).not.toMatch(/\s/);
+      expect(token).not.toContain('--help');
+    }
+    expect(cmd.args.slice(3).length).toBeGreaterThan(0);
+  });
+
   it('environment includes NODE_OPTIONS and does not mutate process.env', () => {
     const adapter = new JavascriptDebugAdapter(deps);
 

packages/shared/src/index.ts

@@ -234,3 +234,11 @@ export {
 export type { SecretRule, RedactionHit, RedactionResult } from './utils/secret-redaction.js';
 export { LineBuffer } from './utils/line-buffer.js';
 export { toSourceBreakpoint, type BreakpointFields, toFunctionBreakpoint, type FunctionBreakpointFields } from './utils/to-source-breakpoint.js';
+// Argv marker constants shared by spawn-time tagging and the startup orphan
+// reapers (issues #343, #431).
+export {
+  PROXY_BOOTSTRAP_MARKER,
+  JS_DEBUG_ADAPTER_MARKER,
+  OWNER_PID_ARG_PREFIX,
+  SESSION_ID_ARG_PREFIX
+} from './utils/process-markers.js';

packages/shared/src/utils/process-markers.ts

@@ -0,0 +1,31 @@
+/**
+ * Argv marker constants shared between the processes that tag child argv at
+ * spawn time (the server's proxy launcher, adapter packages) and the startup
+ * reapers that later recognize those tags in system-wide process scans
+ * (issues #343, #431).
+ *
+ * Marker constraints, imposed by the scan/matcher machinery:
+ * - Tokens must be whitespace-free: the win32 scan splits a process's
+ *   CommandLine on whitespace, so a marker containing a space would fragment
+ *   and never match.
+ * - Identity markers are matched with `String.includes` against every token
+ *   (paths may fragment), so they must be substrings that cannot appear in
+ *   unrelated cmdlines by accident.
+ * - No token may be or contain `--help`: vsDebugServer.cjs prints usage and
+ *   exits when `--help` appears anywhere in its argv.
+ */
+
+/** Substring of the proxy worker's script-path argv token used as its identity marker. */
+export const PROXY_BOOTSTRAP_MARKER = 'proxy-bootstrap';
+
+/**
+ * Substring of the js-debug DAP server's script-path argv token used as its
+ * identity marker (issue #431). On its own this also matches VS Code's own
+ * js-debug instances — matchers must additionally require the owner-pid
+ * marker below, which only our spawns carry.
+ */
+export const JS_DEBUG_ADAPTER_MARKER = 'vsDebugServer';
+
+/** Records the PID of the mcp-debugger server that owned the session. */
+export const OWNER_PID_ARG_PREFIX = '--mcp-owner-pid=';
+export const SESSION_ID_ARG_PREFIX = '--mcp-session-id=';

src/implementations/process-launcher-impl.ts

@@ -32,7 +32,9 @@ class ProxyProcessAdapter extends EventEmitter implements IProxyProcess {
 
   constructor(
     public readonly childProcess: IChildProcess,
-    public readonly sessionId: string
+    public readonly sessionId: string,
+    private readonly platform: NodeJS.Platform = process.platform,
+    private readonly treeKill?: (pid: number) => void
   ) {
     super();
 
@@ -377,12 +379,32 @@ class ProxyProcessAdapter extends EventEmitter implements IProxyProcess {
     if (this.killed || this.disposed) {
       return false; // Already killed or disposed
     }
-    
+
     // If waiting for initialization, fail it
     if (this.initializationState === 'waiting') {
       this.failInitialization(new Error('Process killed during initialization'));
     }
-
+
+    // win32: any cross-process signal is TerminateProcess on the single PID —
+    // the worker's JS never runs, so its own adapter tree-kill cascade cannot
+    // fire, stranding detached adapter children like js-debug's vsDebugServer
+    // (issue #431). Sweep the tree FIRST, while the worker is still alive:
+    // taskkill /T can only discover children through a live parent. Skipped
+    // once the worker has exited — its PID may already be recycled.
+    if (
+      this.platform === 'win32' &&
+      this.treeKill &&
+      this._exitCode === null &&
+      this._signalCode === null &&
+      typeof this.childProcess.pid === 'number'
+    ) {
+      try {
+        this.treeKill(this.childProcess.pid);
+      } catch {
+        // Best-effort: fall through to the plain kill below.
+      }
+    }
+
     try {
       return this.childProcess.kill(signal);
     } catch {
@@ -396,7 +418,9 @@ class ProxyProcessAdapter extends EventEmitter implements IProxyProcess {
  */
 export class ProxyProcessLauncherImpl implements IProxyProcessLauncher {
   constructor(
-    private processManager: IProcessManager
+    private processManager: IProcessManager,
+    private platform: NodeJS.Platform = process.platform,
+    private treeKill?: (pid: number) => void
   ) {}
 
   launchProxy(
@@ -474,8 +498,21 @@ export class ProxyProcessLauncherImpl implements IProxyProcessLauncher {
       options
     );
 
-    // Create the proxy process adapter with the raw child process
-    return new ProxyProcessAdapter(childProcess, sessionId);
+    // Create the proxy process adapter with the raw child process. The
+    // win32 tree-kill (issue #431) defaults to spawning taskkill through the
+    // same process manager; injectable for tests.
+    const treeKill =
+      this.treeKill ??
+      ((pid: number) => {
+        const killer = this.processManager.spawn(
+          'taskkill',
+          ['/PID', String(pid), '/T', '/F'],
+          { stdio: 'ignore', windowsHide: true } as IProcessOptions
+        );
+        // Fire-and-forget: a missing taskkill must not crash the server.
+        killer.on('error', () => {});
+      });
+    return new ProxyProcessAdapter(childProcess, sessionId, this.platform, treeKill);
   }
 
 }

src/utils/proxy-orphan-reaper.ts

@@ -32,15 +32,21 @@
  */
 import { execFile } from 'node:child_process';
 import { promisify } from 'node:util';
+import {
+  PROXY_BOOTSTRAP_MARKER,
+  JS_DEBUG_ADAPTER_MARKER,
+  OWNER_PID_ARG_PREFIX,
+  SESSION_ID_ARG_PREFIX
+} from '@debugmcp/shared';
 import { isPidAlive, SignalFn } from './jvm-orphan-reaper.js';
 import { scanLinux, scanDarwin, scanWindows, scanProcessArgs, type ScannedProcess } from './process-scan.js';
 
 const execFileAsync = promisify(execFile);
 
-/** Substring of the worker's script-path argv token used as the identity marker. */
-export const PROXY_BOOTSTRAP_MARKER = 'proxy-bootstrap';
-export const OWNER_PID_ARG_PREFIX = '--mcp-owner-pid=';
-export const SESSION_ID_ARG_PREFIX = '--mcp-session-id=';
+// Marker constants live in @debugmcp/shared so adapter packages can tag their
+// own spawns with the same markers this reaper matches (issue #431); re-exported
+// here to keep this module the reaper-side import point.
+export { PROXY_BOOTSTRAP_MARKER, JS_DEBUG_ADAPTER_MARKER, OWNER_PID_ARG_PREFIX, SESSION_ID_ARG_PREFIX };
 
 const LIST_TIMEOUT_MS = 5000;
 /** How often the POSIX escalation path polls for the SIGTERM cascade to finish. */
@@ -171,18 +177,17 @@ export async function listWindowsProxies(): Promise<TaggedProxy[]> {
 }
 
 /**
- * Two-factor identification: a token containing the proxy-bootstrap script
- * name AND a valid --mcp-owner-pid marker. Untagged bootstrap processes
- * (pre-upgrade workers) and malformed tags are ignored, never killed.
- *
- * @internal Exposed for unit tests; not part of the public module API.
+ * Two-factor identification: a token containing the identity marker AND a
+ * valid --mcp-owner-pid marker. Untagged processes (pre-upgrade workers,
+ * VS Code's own js-debug instances) and malformed tags are ignored, never
+ * killed.
  */
-export function parseProxyArgs(pid: number, args: string[]): TaggedProxy | null {
+function parseTaggedArgs(pid: number, args: string[], identityMarker: string): TaggedProxy | null {
   let hasMarker = false;
   let ownerPid = -1;
   let sessionId = '';
   for (const a of args) {
-    if (a.includes(PROXY_BOOTSTRAP_MARKER)) {
+    if (a.includes(identityMarker)) {
       hasMarker = true;
     } else if (a.startsWith(OWNER_PID_ARG_PREFIX)) {
       const v = Number(a.slice(OWNER_PID_ARG_PREFIX.length));
@@ -195,6 +200,24 @@ export function parseProxyArgs(pid: number, args: string[]): TaggedProxy | null
   return { pid, ownerPid, sessionId };
 }
 
+/** @internal Exposed for unit tests; not part of the public module API. */
+export function parseProxyArgs(pid: number, args: string[]): TaggedProxy | null {
+  return parseTaggedArgs(pid, args, PROXY_BOOTSTRAP_MARKER);
+}
+
+/**
+ * Matcher for js-debug DAP server processes (vsDebugServer.cjs) tagged at
+ * spawn time by JavascriptDebugAdapter.buildAdapterCommand (issue #431).
+ * These are spawned detached+unref'd by the proxy worker, so a hard-killed
+ * worker strands them outside every tree-kill path; the startup janitor
+ * feeds scan rows through this matcher to find them.
+ *
+ * @internal Exposed for unit tests; not part of the public module API.
+ */
+export function parseJsDebugAdapterArgs(pid: number, args: string[]): TaggedProxy | null {
+  return parseTaggedArgs(pid, args, JS_DEBUG_ADAPTER_MARKER);
+}
+
 const defaultSignal: SignalFn = (pid, signal) => process.kill(pid, signal);
 
 /** Injectable so escalation tests run instantly instead of really sleeping. */

src/utils/startup-janitor.ts

@@ -5,8 +5,9 @@
  * transport from coming up.
  *
  * Covers:
- * - Orphan debuggee JVMs and proxy workers, via ONE shared process scan
- *   feeding both reapers' matchers (previously two independent walks).
+ * - Orphan debuggee JVMs, proxy workers, and js-debug DAP servers (#431), via
+ *   ONE shared process scan feeding all reaper matchers (previously
+ *   independent walks).
  *   `MCP_SKIP_ORPHAN_REAPERS=1` skips this part for PID-namespaced containers
  *   where orphans are impossible.
  * - Stale per-session run logs (proxy-<id>.log / dap-trace-<id>.ndjson) under
@@ -22,7 +23,7 @@ import os from 'node:os';
 import path from 'node:path';
 import { scanProcessArgs, type ProcessScanOptions, type ScannedProcess } from './process-scan.js';
 import { reapOrphanJvms, parseArgs, type ReapResult as JvmReapResult, type ReapOptions as JvmReapOptions } from './jvm-orphan-reaper.js';
-import { reapOrphanProxies, parseProxyArgs, type ReapResult as ProxyReapResult, type ReapOptions as ProxyReapOptions } from './proxy-orphan-reaper.js';
+import { reapOrphanProxies, parseProxyArgs, parseJsDebugAdapterArgs, type ReapResult as ProxyReapResult, type ReapOptions as ProxyReapOptions } from './proxy-orphan-reaper.js';
 
 export interface JanitorLogger {
   info?: (msg: string) => void;
@@ -39,6 +40,7 @@ export interface StartupJanitorOptions {
   scan?: (options: ProcessScanOptions) => Promise<ScannedProcess[]>;
   reapJvms?: (opts: JvmReapOptions) => Promise<JvmReapResult>;
   reapProxies?: (opts: ProxyReapOptions) => Promise<ProxyReapResult>;
+  reapJsDebugAdapters?: (opts: ProxyReapOptions) => Promise<ProxyReapResult>;
   sweep?: (opts: SweepOptions) => Promise<SweepResult>;
 }
 
@@ -77,16 +79,24 @@ async function runOrphanReapers(opts: StartupJanitorOptions, selfPid: number): P
     return;
   }
 
-  // Matchers are marker-based, so feeding every row to both is harmless —
-  // JVM -D markers only appear in java cmdlines and vice versa.
+  // Matchers are marker-based, so feeding every row to all three is harmless —
+  // each identity marker only appears in its own family's cmdlines.
   const jvmLister = async () =>
     processes.map((p) => parseArgs(p.pid, p.args)).filter((t): t is NonNullable<typeof t> => t !== null);
   const proxyLister = async () =>
     processes.map((p) => parseProxyArgs(p.pid, p.args)).filter((t): t is NonNullable<typeof t> => t !== null);
-
-  const [jvmOutcome, proxyOutcome] = await Promise.allSettled([
+  // js-debug DAP servers are spawned detached+unref'd, so a hard-killed worker
+  // strands them outside every tree-kill path (issue #431). Reusing the proxy
+  // reaper works because its win32 kill is taskkill /T /F — while the stranded
+  // vsDebugServer is alive, the tree kill also reaches the debuggee/watchdog
+  // children js-debug spawned beneath it.
+  const jsDebugAdapterLister = async () =>
+    processes.map((p) => parseJsDebugAdapterArgs(p.pid, p.args)).filter((t): t is NonNullable<typeof t> => t !== null);
+
+  const [jvmOutcome, proxyOutcome, jsDebugOutcome] = await Promise.allSettled([
     (opts.reapJvms ?? reapOrphanJvms)({ selfPid, logger: log, lister: jvmLister }),
     (opts.reapProxies ?? reapOrphanProxies)({ selfPid, logger: log, lister: proxyLister }),
+    (opts.reapJsDebugAdapters ?? reapOrphanProxies)({ selfPid, logger: log, lister: jsDebugAdapterLister }),
   ]);
 
   if (jvmOutcome.status === 'fulfilled') {
@@ -103,6 +113,13 @@ async function runOrphanReapers(opts: StartupJanitorOptions, selfPid: number): P
   } else {
     log.warn?.(`[startup-janitor] Orphan proxy reaper failed: ${(proxyOutcome.reason as Error)?.message ?? String(proxyOutcome.reason)}`);
   }
+  if (jsDebugOutcome.status === 'fulfilled') {
+    if (jsDebugOutcome.value.killed.length > 0) {
+      log.info?.(`[startup-janitor] Reaped ${jsDebugOutcome.value.killed.length} orphan js-debug adapter(s) from prior runs`);
+    }
+  } else {
+    log.warn?.(`[startup-janitor] Orphan js-debug adapter reaper failed: ${(jsDebugOutcome.reason as Error)?.message ?? String(jsDebugOutcome.reason)}`);
+  }
 }
 
 /** Delete stale per-session run log dirs after this long (matches logger.ts). */