From 527fef2ed619d81e4e8ac1047b30827c1bf0264d Mon Sep 17 00:00:00 2001 From: JF Date: Sat, 22 Aug 2026 18:49:32 -0400 Subject: [PATCH 1/6] feat(adapters): environment probes for mcp-debugger doctor (#423) - codelldb-common: resolveCodeLLDBExecutableWithSource() attributes which resolution stage won (vendored / env:CODELLDB_PATH / platform-package); resolveCodeLLDBExecutable() now delegates to it. Rust + cpp factories report codelldbSource in validate() details. - adapter-python: getDebugpyVersion() replaces the factory-private boolean probe (same spawn); validate() details gain debugpyVersion. - Doctor-only helpers (not called from validate(), so registration and list_supported_languages cost is unchanged): getNetcoredbgVersion() + getDotnetSdkVersion() (dotnet), findJavacExecutable() (java), getCompilerInfo() (cpp). Co-Authored-By: Claude Fable 5 --- .../adapter-cpp/src/cpp-adapter-factory.ts | 9 ++- packages/adapter-cpp/src/index.ts | 2 + .../adapter-cpp/src/utils/compile-utils.ts | 39 ++++++++++ .../adapter-cpp/tests/compile-utils.test.ts | 43 ++++++++++- .../adapter-cpp/tests/cpp-adapter.test.ts | 19 ++++- packages/adapter-dotnet/src/index.ts | 2 + .../adapter-dotnet/src/utils/dotnet-utils.ts | 61 +++++++++++++++ .../tests/unit/dotnet-utils.test.ts | 70 ++++++++++++++++++ packages/adapter-java/src/index.ts | 2 +- packages/adapter-java/src/utils/java-utils.ts | 62 ++++++++++++++++ packages/adapter-python/src/index.ts | 1 + .../src/python-adapter-factory.ts | 39 +++------- .../adapter-python/src/utils/python-utils.ts | 23 ++++++ .../tests/unit/python-adapter-factory.test.ts | 69 +++-------------- .../unit/python-utils.comprehensive.test.ts | 46 ++++++++++++ packages/adapter-rust/src/index.ts | 2 +- .../adapter-rust/src/rust-adapter-factory.ts | 11 ++- .../src/utils/codelldb-resolver.ts | 3 +- .../adapter-rust/tests/rust-adapter.test.ts | 17 ++++- .../codelldb-common/src/codelldb-resolver.ts | 30 ++++++-- packages/codelldb-common/src/index.ts | 3 +- .../tests/codelldb-resolver.test.ts | 74 +++++++++++++++++++ tests/adapters/java/unit/java-utils.test.ts | 53 +++++++++++++ 23 files changed, 570 insertions(+), 110 deletions(-) diff --git a/packages/adapter-cpp/src/cpp-adapter-factory.ts b/packages/adapter-cpp/src/cpp-adapter-factory.ts index b93277a6..fd240642 100644 --- a/packages/adapter-cpp/src/cpp-adapter-factory.ts +++ b/packages/adapter-cpp/src/cpp-adapter-factory.ts @@ -8,7 +8,7 @@ import { IDebugAdapter } from '@debugmcp/shared'; import { IAdapterFactory, AdapterDependencies, AdapterMetadata, FactoryValidationResult } from '@debugmcp/shared'; import { CppDebugAdapter } from './cpp-debug-adapter.js'; import { DebugLanguage } from '@debugmcp/shared'; -import { resolveCodeLLDBExecutable, getCodeLLDBVersion } from '@debugmcp/codelldb-common'; +import { resolveCodeLLDBExecutableWithSource, getCodeLLDBVersion } from '@debugmcp/codelldb-common'; import { findAnyCompiler } from './utils/compile-utils.js'; /** @@ -49,14 +49,16 @@ export class CppAdapterFactory implements IAdapterFactory { const warnings: string[] = []; let codelldbPath: string | undefined; let codelldbVersion: string | undefined; + let codelldbSource: string | undefined; let compiler: string | undefined; // Check CodeLLDB — the only hard requirement - const resolvedCodelldb = await resolveCodeLLDBExecutable(); + const resolvedCodelldb = await resolveCodeLLDBExecutableWithSource(); if (!resolvedCodelldb) { errors.push('CodeLLDB not found. It normally ships via the @debugmcp/codelldb-* optional dependencies; set CODELLDB_PATH, or in a repo checkout run: npm run build:adapter'); } else { - codelldbPath = resolvedCodelldb; + codelldbPath = resolvedCodelldb.path; + codelldbSource = resolvedCodelldb.source; codelldbVersion = await getCodeLLDBVersion() || undefined; } @@ -75,6 +77,7 @@ export class CppAdapterFactory implements IAdapterFactory { details: { codelldbPath, codelldbVersion, + codelldbSource, compiler, platform: process.platform, arch: process.arch, diff --git a/packages/adapter-cpp/src/index.ts b/packages/adapter-cpp/src/index.ts index 07533054..26dbac59 100644 --- a/packages/adapter-cpp/src/index.ts +++ b/packages/adapter-cpp/src/index.ts @@ -15,7 +15,9 @@ export { dialectForSource, findCompiler, findAnyCompiler, + getCompilerInfo, getDefaultOutputPath, needsRecompile, compileSourceFile } from './utils/compile-utils.js'; +export type { CompilerInfo } from './utils/compile-utils.js'; diff --git a/packages/adapter-cpp/src/utils/compile-utils.ts b/packages/adapter-cpp/src/utils/compile-utils.ts index 6e2872ed..4c06857d 100644 --- a/packages/adapter-cpp/src/utils/compile-utils.ts +++ b/packages/adapter-cpp/src/utils/compile-utils.ts @@ -73,6 +73,45 @@ export async function findAnyCompiler(): Promise { return (await findCompiler('cpp')) ?? (await findCompiler('c')); } +export interface CompilerInfo { + command: string; + version: string | null; +} + +/** + * Discover the available compiler and its version banner (the first line of + * `--version` output, e.g. "g++ (MinGW-w64 ...) 13.2.0"). Doctor-only probe + * (issue #423) — validate() keeps its cheaper presence-only findAnyCompiler. + */ +export async function getCompilerInfo(): Promise { + const command = await findAnyCompiler(); + if (!command) { + return null; + } + return { command, version: await captureVersionLine(command) }; +} + +function captureVersionLine(command: string): Promise { + return new Promise((resolve) => { + try { + const child = spawn(command, ['--version'], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true + }); + let output = ''; + child.stdout?.on('data', (data) => { output += data.toString(); }); + child.stderr?.on('data', (data) => { output += data.toString(); }); + child.on('error', () => resolve(null)); + child.on('exit', (code) => { + const firstLine = output.split(/\r?\n/).map((line) => line.trim()).find((line) => line.length > 0); + resolve(code === 0 && firstLine ? firstLine : null); + }); + } catch { + resolve(null); + } + }); +} + /** * Deterministic output location for a compiled single-file program: * `/.debug-mcp/[.exe]`. Git-ignorable and stable, so diff --git a/packages/adapter-cpp/tests/compile-utils.test.ts b/packages/adapter-cpp/tests/compile-utils.test.ts index a0895a6a..9c7e35fd 100644 --- a/packages/adapter-cpp/tests/compile-utils.test.ts +++ b/packages/adapter-cpp/tests/compile-utils.test.ts @@ -22,12 +22,13 @@ import { dialectForSource, findCompiler, findAnyCompiler, + getCompilerInfo, getDefaultOutputPath, needsRecompile, compileSourceFile } from '../src/utils/compile-utils.js'; -function fakeProcess(exitCode: number, stderr = ''): EventEmitter & { stdout: EventEmitter; stderr: EventEmitter } { +function fakeProcess(exitCode: number, stderr = '', stdout = ''): EventEmitter & { stdout: EventEmitter; stderr: EventEmitter } { const proc = new EventEmitter() as EventEmitter & { stdout: EventEmitter; stderr: EventEmitter }; proc.stdout = new EventEmitter(); proc.stderr = new EventEmitter(); @@ -35,11 +36,22 @@ function fakeProcess(exitCode: number, stderr = ''): EventEmitter & { stdout: Ev if (stderr) { proc.stderr.emit('data', Buffer.from(stderr)); } + if (stdout) { + proc.stdout.emit('data', Buffer.from(stdout)); + } proc.emit('exit', exitCode); }); return proc; } +function erroringProcess(): EventEmitter & { stdout: EventEmitter; stderr: EventEmitter } { + const proc = new EventEmitter() as EventEmitter & { stdout: EventEmitter; stderr: EventEmitter }; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + setImmediate(() => proc.emit('error', new Error('ENOENT'))); + return proc; +} + describe('compile-utils', () => { beforeEach(() => { spawnMock.mockReset(); @@ -131,6 +143,35 @@ describe('compile-utils', () => { }); }); + describe('getCompilerInfo (issue #423)', () => { + it('returns the discovered command and the first line of its --version output', async () => { + spawnMock + .mockImplementationOnce(() => fakeProcess(0)) // findAnyCompiler probe: g++ answers + .mockImplementationOnce(() => + fakeProcess(0, '', 'g++ (MinGW-w64 x86_64-posix-seh) 13.2.0\nCopyright (C) 2023 Free Software Foundation\n') + ); + + await expect(getCompilerInfo()).resolves.toEqual({ + command: 'g++', + version: 'g++ (MinGW-w64 x86_64-posix-seh) 13.2.0' + }); + }); + + it('returns null when no compiler is installed', async () => { + spawnMock.mockImplementation(() => fakeProcess(1)); // every candidate probe fails + + await expect(getCompilerInfo()).resolves.toBeNull(); + }); + + it('returns a null version when the --version re-run fails after discovery', async () => { + spawnMock + .mockImplementationOnce(() => fakeProcess(0)) // probe succeeds + .mockImplementationOnce(() => erroringProcess()); // version capture fails + + await expect(getCompilerInfo()).resolves.toEqual({ command: 'g++', version: null }); + }); + }); + describe('getDefaultOutputPath', () => { it('places the binary under .debug-mcp next to the source, with .exe on win32', () => { const src = path.join('C:', 'work', 'demo', 'hello.cpp'); diff --git a/packages/adapter-cpp/tests/cpp-adapter.test.ts b/packages/adapter-cpp/tests/cpp-adapter.test.ts index fdbe8025..995837e1 100644 --- a/packages/adapter-cpp/tests/cpp-adapter.test.ts +++ b/packages/adapter-cpp/tests/cpp-adapter.test.ts @@ -13,6 +13,7 @@ import type { AdapterConfig, AdapterDependencies } from '@debugmcp/shared'; vi.mock('@debugmcp/codelldb-common', async (importOriginal) => ({ ...(await importOriginal()), resolveCodeLLDBExecutable: vi.fn(), + resolveCodeLLDBExecutableWithSource: vi.fn(), resolveCodeLLDBExecutableSyncImpl: vi.fn(), getCodeLLDBVersion: vi.fn(), detectBinaryFormat: vi.fn(), @@ -29,6 +30,7 @@ vi.mock('../src/utils/compile-utils.js', async (importOriginal) => ({ import { resolveCodeLLDBExecutable, + resolveCodeLLDBExecutableWithSource, resolveCodeLLDBExecutableSyncImpl, getCodeLLDBVersion, detectBinaryFormat, @@ -420,6 +422,7 @@ describe('CppDebugAdapter', () => { describe('CppAdapterFactory', () => { beforeEach(() => { vi.mocked(resolveCodeLLDBExecutable).mockReset(); + vi.mocked(resolveCodeLLDBExecutableWithSource).mockReset(); vi.mocked(getCodeLLDBVersion).mockReset(); vi.mocked(findAnyCompiler).mockReset(); }); @@ -436,9 +439,12 @@ describe('CppAdapterFactory', () => { expect(adapter.language).toBe(DebugLanguage.CPP); }); - it('validate reports the discovered compiler in details when everything is present', async () => { + it('validate reports the discovered compiler and CodeLLDB source in details when everything is present', async () => { const factory = new CppAdapterFactory(); - vi.mocked(resolveCodeLLDBExecutable).mockResolvedValue('/vendor/codelldb'); + vi.mocked(resolveCodeLLDBExecutableWithSource).mockResolvedValue({ + path: '/vendor/codelldb', + source: 'vendored' + }); vi.mocked(getCodeLLDBVersion).mockResolvedValue('1.11.8'); vi.mocked(findAnyCompiler).mockResolvedValue('clang++'); @@ -449,6 +455,7 @@ describe('CppAdapterFactory', () => { expect(result.details).toMatchObject({ codelldbPath: '/vendor/codelldb', codelldbVersion: '1.11.8', + codelldbSource: 'vendored', compiler: 'clang++' }); }); @@ -456,17 +463,21 @@ describe('CppAdapterFactory', () => { it('validate errors without CodeLLDB and warns without a compiler', async () => { const factory = new CppAdapterFactory(); - vi.mocked(resolveCodeLLDBExecutable).mockResolvedValue(null); + vi.mocked(resolveCodeLLDBExecutableWithSource).mockResolvedValue(null); vi.mocked(findAnyCompiler).mockResolvedValue(null); let result = await factory.validate(); expect(result.valid).toBe(false); expect(result.errors[0]).toMatch(/CodeLLDB/); - vi.mocked(resolveCodeLLDBExecutable).mockResolvedValue('/vendor/codelldb'); + vi.mocked(resolveCodeLLDBExecutableWithSource).mockResolvedValue({ + path: '/vendor/codelldb', + source: 'env:CODELLDB_PATH' + }); vi.mocked(getCodeLLDBVersion).mockResolvedValue('1.11.8'); vi.mocked(findAnyCompiler).mockResolvedValue(null); result = await factory.validate(); expect(result.valid).toBe(true); expect(result.warnings[0]).toMatch(/compiler/i); + expect(result.details).toMatchObject({ codelldbSource: 'env:CODELLDB_PATH' }); }); }); diff --git a/packages/adapter-dotnet/src/index.ts b/packages/adapter-dotnet/src/index.ts index b575b0e6..fb7cd211 100644 --- a/packages/adapter-dotnet/src/index.ts +++ b/packages/adapter-dotnet/src/index.ts @@ -10,6 +10,8 @@ export { DotnetDebugAdapter } from './DotnetDebugAdapter.js'; export { findNetcoredbgExecutable, findDotnetBackend, + getNetcoredbgVersion, + getDotnetSdkVersion, listDotnetProcesses, isPortablePdb, findPdb2PdbExecutable, diff --git a/packages/adapter-dotnet/src/utils/dotnet-utils.ts b/packages/adapter-dotnet/src/utils/dotnet-utils.ts index c268a445..524bd7f1 100644 --- a/packages/adapter-dotnet/src/utils/dotnet-utils.ts +++ b/packages/adapter-dotnet/src/utils/dotnet-utils.ts @@ -200,6 +200,67 @@ export async function findDotnetBackend( return { backend: 'netcoredbg', path: netcoredbgPath }; } +/** + * Report the version of a netcoredbg executable, or null when it cannot be + * spawned or exits non-zero. Extracts the version token from output like + * "NET Core debugger 3.1.2-1054"; falls back to the first non-empty output + * line for builds that print a different banner. Doctor-only probe (issue + * #423) — not called from validate(), so registration cost is unchanged. + */ +export async function getNetcoredbgVersion(netcoredbgPath: string): Promise { + return new Promise((resolve) => { + try { + const child = spawn(netcoredbgPath, ['--version'], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true + }); + let output = ''; + child.stdout?.on('data', (data) => { output += data.toString(); }); + child.on('error', () => resolve(null)); + child.on('exit', (code) => { + if (code !== 0) { + resolve(null); + return; + } + const match = output.match(/(\d+\.\d+\.\d+(?:-[\w.]+)?)/); + if (match) { + resolve(match[1]); + return; + } + const firstLine = output.split(/\r?\n/).map((line) => line.trim()).find((line) => line.length > 0); + resolve(firstLine ?? null); + }); + } catch { + resolve(null); + } + }); +} + +/** + * Report the active .NET SDK version (`dotnet --version`), or null when the + * dotnet CLI is missing or reports an error (e.g. no SDK behind a bare + * runtime install). Doctor-only probe (issue #423). + */ +export async function getDotnetSdkVersion(): Promise { + return new Promise((resolve) => { + try { + const child = spawn('dotnet', ['--version'], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true + }); + let output = ''; + child.stdout?.on('data', (data) => { output += data.toString(); }); + child.on('error', () => resolve(null)); + child.on('exit', (code) => { + const version = output.trim().split(/\r?\n/)[0]?.trim() ?? ''; + resolve(code === 0 && version.length > 0 ? version : null); + }); + } catch { + resolve(null); + } + }); +} + /** * List running .NET processes on the system. * Currently Windows-only using tasklist. diff --git a/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts b/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts index 7a97b0ff..109ee719 100644 --- a/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts +++ b/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts @@ -62,6 +62,8 @@ import { getExeArchitecture, getProcessArchitecture, getProcessExecutablePath, + getNetcoredbgVersion, + getDotnetSdkVersion, CommandNotFoundError } from '../../src/utils/dotnet-utils.js'; @@ -100,6 +102,74 @@ const createSpawn = (options: { exitCode: number; stdout?: string; stderr?: stri return proc; }; +describe('getNetcoredbgVersion (issue #423)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('extracts the version token from netcoredbg --version output', async () => { + spawnMock.mockImplementation(() => + createSpawn({ exitCode: 0, stdout: 'NET Core debugger 3.1.2-1054\n\nCopyright (c) 2020 Samsung Electronics Co., LTD\n' }) + ); + + await expect(getNetcoredbgVersion('/opt/netcoredbg/netcoredbg')).resolves.toBe('3.1.2-1054'); + expect(spawnMock).toHaveBeenCalledWith( + '/opt/netcoredbg/netcoredbg', + ['--version'], + expect.objectContaining({ windowsHide: true }) + ); + }); + + it('falls back to the first non-empty output line when no version token matches', async () => { + spawnMock.mockImplementation(() => + createSpawn({ exitCode: 0, stdout: '\ncustom development build\n' }) + ); + + await expect(getNetcoredbgVersion('/opt/netcoredbg/netcoredbg')).resolves.toBe('custom development build'); + }); + + it('returns null on a non-zero exit code', async () => { + spawnMock.mockImplementation(() => createSpawn({ exitCode: 1, stderr: 'boom' })); + + await expect(getNetcoredbgVersion('/opt/netcoredbg/netcoredbg')).resolves.toBeNull(); + }); + + it('returns null when the executable cannot be spawned', async () => { + spawnMock.mockImplementation(() => createSpawn({ exitCode: 0, error: new Error('ENOENT') })); + + await expect(getNetcoredbgVersion('/missing/netcoredbg')).resolves.toBeNull(); + }); +}); + +describe('getDotnetSdkVersion (issue #423)', () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('returns the trimmed dotnet --version output', async () => { + spawnMock.mockImplementation(() => createSpawn({ exitCode: 0, stdout: '8.0.301\n' })); + + await expect(getDotnetSdkVersion()).resolves.toBe('8.0.301'); + expect(spawnMock).toHaveBeenCalledWith( + 'dotnet', + ['--version'], + expect.objectContaining({ windowsHide: true }) + ); + }); + + it('returns null when the dotnet CLI is not installed', async () => { + spawnMock.mockImplementation(() => createSpawn({ exitCode: 0, error: new Error('ENOENT') })); + + await expect(getDotnetSdkVersion()).resolves.toBeNull(); + }); + + it('returns null on a non-zero exit code', async () => { + spawnMock.mockImplementation(() => createSpawn({ exitCode: 145, stderr: 'A compatible .NET SDK was not found.' })); + + await expect(getDotnetSdkVersion()).resolves.toBeNull(); + }); +}); + describe('CommandNotFoundError', () => { it('creates error with command property', () => { const error = new CommandNotFoundError('netcoredbg'); diff --git a/packages/adapter-java/src/index.ts b/packages/adapter-java/src/index.ts index c4ef2467..77a97df4 100644 --- a/packages/adapter-java/src/index.ts +++ b/packages/adapter-java/src/index.ts @@ -10,7 +10,7 @@ export { JavaDebugAdapter } from './java-debug-adapter.js'; export { JavaAdapterFactory } from './java-adapter-factory.js'; -export { findJavaExecutable, getJavaVersion, getJavaSearchPaths } from './utils/java-utils.js'; +export { findJavaExecutable, findJavacExecutable, getJavaVersion, getJavaSearchPaths } from './utils/java-utils.js'; export { resolveJdiBridgeClassDir, ensureJdiBridgeCompiled } from './utils/jdi-resolver.js'; import { JavaAdapterFactory as _JavaAdapterFactory } from './java-adapter-factory.js'; diff --git a/packages/adapter-java/src/utils/java-utils.ts b/packages/adapter-java/src/utils/java-utils.ts index 0f8e4016..18014d6b 100644 --- a/packages/adapter-java/src/utils/java-utils.ts +++ b/packages/adapter-java/src/utils/java-utils.ts @@ -72,6 +72,68 @@ export async function validateJavaExecutable(javaPath: string): Promise }); } +/** + * Find a working javac executable, or null when none validates. + * + * Priority: sibling of the resolved java path > JAVA_HOME/bin/javac > 'javac' + * in PATH. Doctor-only probe (issue #423): javac is required to compile + * target code with -g for variable inspection, but the adapter itself only + * needs the JRE side, so this is never part of validate(). + */ +export async function findJavacExecutable(javaPath?: string): Promise { + /* istanbul ignore next -- platform-specific executable extension */ + const ext = process.platform === 'win32' ? '.exe' : ''; + const candidates: string[] = []; + + if (javaPath && path.dirname(javaPath) !== '.') { + candidates.push(path.join(path.dirname(javaPath), `javac${ext}`)); + } + if (process.env.JAVA_HOME) { + candidates.push(path.join(process.env.JAVA_HOME, 'bin', `javac${ext}`)); + } + candidates.push('javac'); + + for (const candidate of new Set(candidates)) { + if (await validateJavacExecutable(candidate)) { + return candidate; + } + } + return null; +} + +/** + * Validate that a javac executable works by running `javac -version`. + */ +async function validateJavacExecutable(javacPath: string): Promise { + return new Promise((resolve) => { + let settled = false; + try { + const child = spawn(javacPath, ['-version'], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true, + }); + + let hasOutput = false; + child.stdout?.on('data', () => { hasOutput = true; }); + child.stderr?.on('data', () => { hasOutput = true; }); + child.on('error', () => { + if (settled) return; + settled = true; + resolve(false); + }); + child.on('exit', (code) => { + if (settled) return; + settled = true; + resolve(code === 0 && hasOutput); + }); + } catch { + if (settled) return; + settled = true; + resolve(false); + } + }); +} + /** * Get the Java version string. */ diff --git a/packages/adapter-python/src/index.ts b/packages/adapter-python/src/index.ts index 0082e656..bbcbe356 100644 --- a/packages/adapter-python/src/index.ts +++ b/packages/adapter-python/src/index.ts @@ -10,6 +10,7 @@ export { PythonDebugAdapter } from './python-debug-adapter.js'; export { findPythonExecutable, getPythonVersion, + getDebugpyVersion, setDefaultCommandFinder, resetDefaultCommandFinder, CommandNotFoundError diff --git a/packages/adapter-python/src/python-adapter-factory.ts b/packages/adapter-python/src/python-adapter-factory.ts index 9176d913..2dae5ab8 100644 --- a/packages/adapter-python/src/python-adapter-factory.ts +++ b/packages/adapter-python/src/python-adapter-factory.ts @@ -10,8 +10,7 @@ import { IDebugAdapter } from '@debugmcp/shared'; import { IAdapterFactory, AdapterDependencies, AdapterMetadata, FactoryValidationResult } from '@debugmcp/shared'; import { PythonDebugAdapter } from './python-debug-adapter.js'; import { DebugLanguage } from '@debugmcp/shared'; -import { findPythonExecutable, getPythonVersion } from './utils/python-utils.js'; -import { spawn } from 'child_process'; +import { findPythonExecutable, getPythonVersion, getDebugpyVersion } from './utils/python-utils.js'; /** * Factory for creating Python debug adapters @@ -51,11 +50,12 @@ export class PythonAdapterFactory implements IAdapterFactory { const warnings: string[] = []; let pythonPath: string | undefined; let pythonVersion: string | undefined; - + let debugpyVersion: string | undefined; + try { // Check Python executable pythonPath = await findPythonExecutable(); - + // Check Python version pythonVersion = await getPythonVersion(pythonPath) || undefined; if (pythonVersion) { @@ -66,18 +66,18 @@ export class PythonAdapterFactory implements IAdapterFactory { } else { warnings.push('Could not determine Python version'); } - + // Check debugpy installation (warning only — debugpy may be available in the // user's virtualenv even if missing from system Python. See issue #16.) - const hasDebugpy = await this.checkDebugpyInstalled(pythonPath); - if (!hasDebugpy) { + debugpyVersion = await getDebugpyVersion(pythonPath) || undefined; + if (!debugpyVersion) { warnings.push('debugpy not found in system Python. If using a virtualenv, debugpy will be checked at launch time. Otherwise run: pip install debugpy'); } - + } catch (error) { errors.push(error instanceof Error ? error.message : 'Python executable not found'); } - + return { valid: errors.length === 0, errors, @@ -85,30 +85,11 @@ export class PythonAdapterFactory implements IAdapterFactory { details: { pythonPath, pythonVersion, + debugpyVersion, pythonDetectionMethod: 'multi-strategy', platform: process.platform, timestamp: new Date().toISOString() } }; } - - /** - * Check if debugpy is installed - */ - private checkDebugpyInstalled(pythonPath: string): Promise { - return new Promise((resolve) => { - const child = spawn(pythonPath, ['-c', 'import debugpy; print(debugpy.__version__)'], { - stdio: ['ignore', 'pipe', 'pipe'], - windowsHide: true - }); - - let output = ''; - child.stdout?.on('data', (data) => { output += data.toString(); }); - - child.on('error', () => resolve(false)); - child.on('exit', (code) => { - resolve(code === 0 && output.trim().length > 0); - }); - }); - } } diff --git a/packages/adapter-python/src/utils/python-utils.ts b/packages/adapter-python/src/utils/python-utils.ts index 5528a021..317b76b9 100644 --- a/packages/adapter-python/src/utils/python-utils.ts +++ b/packages/adapter-python/src/utils/python-utils.ts @@ -488,6 +488,29 @@ export async function findPythonExecutable( /** * Get Python version for a given executable */ +/** + * Report the debugpy version importable by the given interpreter, or null when + * debugpy is missing (or the interpreter cannot be spawned). Null is not fatal: + * debugpy may still be available in the user's virtualenv (issue #16). + */ +export async function getDebugpyVersion(pythonPath: string): Promise { + return new Promise((resolve) => { + const child = spawn(pythonPath, ['-c', 'import debugpy; print(debugpy.__version__)'], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true + }); + + let output = ''; + child.stdout?.on('data', (data) => { output += data.toString(); }); + + child.on('error', () => resolve(null)); + child.on('exit', (code) => { + const version = output.trim(); + resolve(code === 0 && version.length > 0 ? version : null); + }); + }); +} + export async function getPythonVersion(pythonPath: string): Promise { return new Promise((resolve) => { const child = spawn(pythonPath, ['--version'], { stdio: 'pipe', windowsHide: true }); diff --git a/packages/adapter-python/tests/unit/python-adapter-factory.test.ts b/packages/adapter-python/tests/unit/python-adapter-factory.test.ts index b25475c6..ca053eca 100644 --- a/packages/adapter-python/tests/unit/python-adapter-factory.test.ts +++ b/packages/adapter-python/tests/unit/python-adapter-factory.test.ts @@ -1,29 +1,19 @@ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import type { Mock } from 'vitest'; import type { AdapterDependencies } from '@debugmcp/shared'; import { DebugLanguage } from '@debugmcp/shared'; -import { EventEmitter } from 'events'; import { PythonAdapterFactory } from '../../src/python-adapter-factory.js'; import { PythonDebugAdapter } from '../../src/python-debug-adapter.js'; -import { findPythonExecutable, getPythonVersion } from '../../src/utils/python-utils.js'; -import { spawn } from 'child_process'; +import { findPythonExecutable, getPythonVersion, getDebugpyVersion } from '../../src/utils/python-utils.js'; vi.mock('../../src/utils/python-utils.js', () => ({ findPythonExecutable: vi.fn(), - getPythonVersion: vi.fn() + getPythonVersion: vi.fn(), + getDebugpyVersion: vi.fn() })); -vi.mock('child_process', async () => { - const actual = await vi.importActual('child_process'); - return { - ...actual, - spawn: vi.fn() - }; -}); - const findPythonExecutableMock = vi.mocked(findPythonExecutable); const getPythonVersionMock = vi.mocked(getPythonVersion); -const spawnMock = spawn as unknown as Mock; +const getDebugpyVersionMock = vi.mocked(getDebugpyVersion); const createDependencies = (): AdapterDependencies & { logger: { info: () => void; debug: () => void; error: () => void }; @@ -41,35 +31,12 @@ const createDependencies = (): AdapterDependencies & { } }); -const simulateSpawn = (options: { output?: string; exitCode?: number; emitError?: boolean } = {}): void => { - const { output = '', exitCode = 0, emitError = false } = options; - spawnMock.mockImplementation(() => { - const stdout = new EventEmitter(); - const child = new EventEmitter() as EventEmitter & { stdout: EventEmitter }; - (child as unknown as { stdout: EventEmitter }).stdout = stdout; - - queueMicrotask(() => { - if (emitError) { - child.emit('error', new Error('spawn failed')); - return; - } - - if (output) { - stdout.emit('data', Buffer.from(output)); - } - child.emit('exit', exitCode ?? 0); - }); - - return child as unknown as ReturnType; - }); -}; - describe('PythonAdapterFactory', () => { beforeEach(() => { vi.clearAllMocks(); findPythonExecutableMock.mockReset(); getPythonVersionMock.mockReset(); - spawnMock.mockReset(); + getDebugpyVersionMock.mockReset(); }); it('creates PythonDebugAdapter instances with provided dependencies', () => { @@ -97,7 +64,7 @@ describe('PythonAdapterFactory', () => { it('validates environment when Python and debugpy are available', async () => { findPythonExecutableMock.mockResolvedValue('/usr/bin/python3'); getPythonVersionMock.mockResolvedValue('3.10.1'); - simulateSpawn({ output: '1.8.1', exitCode: 0 }); + getDebugpyVersionMock.mockResolvedValue('1.8.1'); const factory = new PythonAdapterFactory(); const result = await factory.validate(); @@ -108,6 +75,7 @@ describe('PythonAdapterFactory', () => { expect(result.details).toMatchObject({ pythonPath: '/usr/bin/python3', pythonVersion: '3.10.1', + debugpyVersion: '1.8.1', platform: process.platform }); }); @@ -125,7 +93,7 @@ describe('PythonAdapterFactory', () => { it('reports error when Python version is below 3.7', async () => { findPythonExecutableMock.mockResolvedValue('/usr/bin/python3'); getPythonVersionMock.mockResolvedValue('3.6.9'); - simulateSpawn({ output: '1.6.0', exitCode: 0 }); + getDebugpyVersionMock.mockResolvedValue('1.6.0'); const factory = new PythonAdapterFactory(); const result = await factory.validate(); @@ -137,7 +105,7 @@ describe('PythonAdapterFactory', () => { it('warns when Python version cannot be determined', async () => { findPythonExecutableMock.mockResolvedValue('/usr/bin/python3'); getPythonVersionMock.mockResolvedValue(undefined); - simulateSpawn({ output: '1.6.0', exitCode: 0 }); + getDebugpyVersionMock.mockResolvedValue('1.6.0'); const factory = new PythonAdapterFactory(); const result = await factory.validate(); @@ -147,23 +115,10 @@ describe('PythonAdapterFactory', () => { expect(result.warnings).toContain('Could not determine Python version'); }); - it('warns (not errors) when debugpy detection fails with exit code', async () => { - findPythonExecutableMock.mockResolvedValue('/usr/bin/python3'); - getPythonVersionMock.mockResolvedValue('3.10.1'); - simulateSpawn({ output: '', exitCode: 1 }); - - const factory = new PythonAdapterFactory(); - const result = await factory.validate(); - - expect(result.valid).toBe(true); - expect(result.errors).toEqual([]); - expect(result.warnings.some(w => w.includes('debugpy'))).toBe(true); - }); - - it('warns (not errors) when debugpy spawn emits an error', async () => { + it('warns (not errors) when debugpy detection fails', async () => { findPythonExecutableMock.mockResolvedValue('/usr/bin/python3'); getPythonVersionMock.mockResolvedValue('3.10.1'); - simulateSpawn({ emitError: true }); + getDebugpyVersionMock.mockResolvedValue(null); const factory = new PythonAdapterFactory(); const result = await factory.validate(); @@ -179,7 +134,7 @@ describe('PythonAdapterFactory', () => { // return valid:true with a warning, NOT block adapter registration. findPythonExecutableMock.mockResolvedValue('/usr/bin/python3'); getPythonVersionMock.mockResolvedValue('3.11.0'); - simulateSpawn({ output: '', exitCode: 1 }); // debugpy not installed + getDebugpyVersionMock.mockResolvedValue(null); // debugpy not installed const factory = new PythonAdapterFactory(); const result = await factory.validate(); diff --git a/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts b/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts index 5e0cd755..c4b0e4ff 100644 --- a/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts +++ b/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts @@ -20,6 +20,7 @@ import which from 'which'; import { findPythonExecutable, getPythonVersion, + getDebugpyVersion, setDefaultCommandFinder, resetDefaultCommandFinder, CommandNotFoundError, @@ -558,6 +559,51 @@ describe('getPythonVersion', () => { }); }); +describe('getDebugpyVersion (issue #423)', () => { + beforeEach(() => { + spawnMock.mockReset(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('returns the printed debugpy version, trimmed', async () => { + spawnMock.mockImplementation(() => + createSpawn({ exitCode: 0, stdout: '1.8.14\n' }) + ); + + await expect(getDebugpyVersion('/usr/bin/python3')).resolves.toBe('1.8.14'); + expect(spawnMock).toHaveBeenCalledWith( + '/usr/bin/python3', + ['-c', 'import debugpy; print(debugpy.__version__)'], + expect.objectContaining({ windowsHide: true }) + ); + }); + + it('returns null when the import fails (debugpy not installed)', async () => { + spawnMock.mockImplementation(() => + createSpawn({ exitCode: 1, stderr: "ModuleNotFoundError: No module named 'debugpy'" }) + ); + + await expect(getDebugpyVersion('/usr/bin/python3')).resolves.toBeNull(); + }); + + it('returns null when the interpreter cannot be spawned', async () => { + spawnMock.mockImplementation(() => + createSpawn({ exitCode: 0, error: new Error('ENOENT') }) + ); + + await expect(getDebugpyVersion('/nonexistent/python')).resolves.toBeNull(); + }); + + it('returns null when the command prints nothing', async () => { + spawnMock.mockImplementation(() => createSpawn({ exitCode: 0 })); + + await expect(getDebugpyVersion('/usr/bin/python3')).resolves.toBeNull(); + }); +}); + describe('WhichCommandFinder class behavior', () => { beforeEach(() => { spawnMock.mockReset(); diff --git a/packages/adapter-rust/src/index.ts b/packages/adapter-rust/src/index.ts index fff4a8bc..78d4e91e 100644 --- a/packages/adapter-rust/src/index.ts +++ b/packages/adapter-rust/src/index.ts @@ -10,6 +10,6 @@ export { RustDebugAdapter } from './rust-debug-adapter.js'; export { RustAdapterFactory } from './rust-adapter-factory.js'; export { resolveCodeLLDBPath, checkCargoInstallation } from './utils/rust-utils.js'; export { resolveCargoProject, getCargoTargets } from './utils/cargo-utils.js'; -export { resolveCodeLLDBExecutable } from './utils/codelldb-resolver.js'; +export { resolveCodeLLDBExecutable, resolveCodeLLDBExecutableWithSource } from './utils/codelldb-resolver.js'; export { detectBinaryFormat } from './utils/binary-detector.js'; export type { BinaryInfo } from './utils/binary-detector.js'; diff --git a/packages/adapter-rust/src/rust-adapter-factory.ts b/packages/adapter-rust/src/rust-adapter-factory.ts index d2870211..f9e57415 100644 --- a/packages/adapter-rust/src/rust-adapter-factory.ts +++ b/packages/adapter-rust/src/rust-adapter-factory.ts @@ -9,7 +9,7 @@ import { IAdapterFactory, AdapterDependencies, AdapterMetadata, FactoryValidatio import { RustDebugAdapter } from './rust-debug-adapter.js'; import { DebugLanguage } from '@debugmcp/shared'; import { checkCargoInstallation, getCargoVersion, getRustHostTriple } from './utils/rust-utils.js'; -import { resolveCodeLLDBExecutable, getCodeLLDBVersion } from './utils/codelldb-resolver.js'; +import { resolveCodeLLDBExecutableWithSource, getCodeLLDBVersion } from './utils/codelldb-resolver.js'; /** * Factory for creating Rust debug adapters @@ -49,15 +49,17 @@ export class RustAdapterFactory implements IAdapterFactory { const warnings: string[] = []; let codelldbPath: string | undefined; let codelldbVersion: string | undefined; + let codelldbSource: string | undefined; let cargoVersion: string | undefined; let hostTriple: string | undefined; - + // Check CodeLLDB - const resolvedCodelldb = await resolveCodeLLDBExecutable(); + const resolvedCodelldb = await resolveCodeLLDBExecutableWithSource(); if (!resolvedCodelldb) { errors.push('CodeLLDB not found. It normally ships via the @debugmcp/codelldb-* optional dependencies; set CODELLDB_PATH, or in a repo checkout run: npm run build:adapter'); } else { - codelldbPath = resolvedCodelldb; + codelldbPath = resolvedCodelldb.path; + codelldbSource = resolvedCodelldb.source; codelldbVersion = await getCodeLLDBVersion() || undefined; } @@ -84,6 +86,7 @@ export class RustAdapterFactory implements IAdapterFactory { details: { codelldbPath, codelldbVersion, + codelldbSource, cargoVersion, hostTriple, platform: process.platform, diff --git a/packages/adapter-rust/src/utils/codelldb-resolver.ts b/packages/adapter-rust/src/utils/codelldb-resolver.ts index 12e61132..d15b44ea 100644 --- a/packages/adapter-rust/src/utils/codelldb-resolver.ts +++ b/packages/adapter-rust/src/utils/codelldb-resolver.ts @@ -15,6 +15,7 @@ export { buildVendorCandidatePaths, resolveCodeLLDBExecutableSyncImpl, resolveCodeLLDBExecutable, + resolveCodeLLDBExecutableWithSource, getCodeLLDBVersion } from '@debugmcp/codelldb-common'; -export type { CodeLLDBPlatformDir } from '@debugmcp/codelldb-common'; +export type { CodeLLDBPlatformDir, CodeLLDBSource } from '@debugmcp/codelldb-common'; diff --git a/packages/adapter-rust/tests/rust-adapter.test.ts b/packages/adapter-rust/tests/rust-adapter.test.ts index 7fad85ea..53c50194 100644 --- a/packages/adapter-rust/tests/rust-adapter.test.ts +++ b/packages/adapter-rust/tests/rust-adapter.test.ts @@ -20,6 +20,10 @@ import * as path from 'path'; vi.mock('../src/utils/codelldb-resolver.js', async (importOriginal) => ({ ...(await importOriginal()), resolveCodeLLDBExecutable: vi.fn(async () => '/mock/vendor/codelldb'), + resolveCodeLLDBExecutableWithSource: vi.fn(async () => ({ + path: '/mock/vendor/codelldb', + source: 'vendored' + })), getCodeLLDBVersion: vi.fn(async () => '1.11.0') })); vi.mock('../src/utils/rust-utils.js', async (importOriginal) => ({ @@ -387,10 +391,19 @@ describe('RustAdapterFactory', () => { }); }); + it('should attribute the CodeLLDB resolution source in details (issue #423)', async () => { + const result = await factory.validate(); + + expect(result.details).toMatchObject({ + codelldbPath: '/mock/vendor/codelldb', + codelldbSource: 'vendored' + }); + }); + it('should report an error when CodeLLDB is missing and a warning when cargo is missing', async () => { - const { resolveCodeLLDBExecutable } = await import('../src/utils/codelldb-resolver.js'); + const { resolveCodeLLDBExecutableWithSource } = await import('../src/utils/codelldb-resolver.js'); const { checkCargoInstallation } = await import('../src/utils/rust-utils.js'); - vi.mocked(resolveCodeLLDBExecutable).mockResolvedValueOnce(null); + vi.mocked(resolveCodeLLDBExecutableWithSource).mockResolvedValueOnce(null); vi.mocked(checkCargoInstallation).mockResolvedValueOnce(false); const result = await factory.validate(); diff --git a/packages/codelldb-common/src/codelldb-resolver.ts b/packages/codelldb-common/src/codelldb-resolver.ts index 669f42dd..27d845f8 100644 --- a/packages/codelldb-common/src/codelldb-resolver.ts +++ b/packages/codelldb-common/src/codelldb-resolver.ts @@ -201,11 +201,19 @@ export function resolveCodeLLDBExecutableSyncImpl(options?: { } /** - * Resolve the CodeLLDB executable path based on platform + * Which resolution stage produced the CodeLLDB executable (issue #423). + * Reported by `mcp-debugger doctor` so users can see whether they are running + * a vendored copy, an explicit CODELLDB_PATH override, or a platform package. */ -export async function resolveCodeLLDBExecutable(options?: { +export type CodeLLDBSource = 'vendored' | 'env:CODELLDB_PATH' | 'platform-package'; + +/** + * Resolve the CodeLLDB executable and report which stage of the documented + * order (vendored → CODELLDB_PATH → platform package) produced it. + */ +export async function resolveCodeLLDBExecutableWithSource(options?: { resolvePlatformPackageRoot?: (platformDir: string) => string | null; -}): Promise { +}): Promise<{ path: string; source: CodeLLDBSource } | null> { const resolvePlatformPackageRoot = options?.resolvePlatformPackageRoot ?? resolveCodeLLDBPlatformPackageRoot; @@ -224,7 +232,7 @@ export async function resolveCodeLLDBExecutable(options?: { for (const candidate of candidatePaths) { try { await fs.access(candidate, fsConstants.F_OK); - return candidate; + return { path: candidate, source: 'vendored' }; } catch { // Try next candidate } @@ -236,7 +244,7 @@ export async function resolveCodeLLDBExecutable(options?: { if (process.env.CODELLDB_PATH) { try { await fs.access(process.env.CODELLDB_PATH, fsConstants.F_OK); - return process.env.CODELLDB_PATH; + return { path: process.env.CODELLDB_PATH, source: 'env:CODELLDB_PATH' }; } catch { // Fall through } @@ -250,7 +258,7 @@ export async function resolveCodeLLDBExecutable(options?: { const candidate = path.join(platformPackageRoot, 'adapter', getCodeLLDBExecutableName(process.platform)); try { await fs.access(candidate, fsConstants.F_OK); - return candidate; + return { path: candidate, source: 'platform-package' }; } catch { // Fall through } @@ -259,6 +267,16 @@ export async function resolveCodeLLDBExecutable(options?: { return null; } +/** + * Resolve the CodeLLDB executable path based on platform + */ +export async function resolveCodeLLDBExecutable(options?: { + resolvePlatformPackageRoot?: (platformDir: string) => string | null; +}): Promise { + const resolved = await resolveCodeLLDBExecutableWithSource(options); + return resolved?.path ?? null; +} + /** * Check if CodeLLDB is installed and get version */ diff --git a/packages/codelldb-common/src/index.ts b/packages/codelldb-common/src/index.ts index 37158c05..9d10060e 100644 --- a/packages/codelldb-common/src/index.ts +++ b/packages/codelldb-common/src/index.ts @@ -17,9 +17,10 @@ export { buildVendorCandidatePaths, resolveCodeLLDBExecutableSyncImpl, resolveCodeLLDBExecutable, + resolveCodeLLDBExecutableWithSource, getCodeLLDBVersion } from './codelldb-resolver.js'; -export type { CodeLLDBPlatformDir } from './codelldb-resolver.js'; +export type { CodeLLDBPlatformDir, CodeLLDBSource } from './codelldb-resolver.js'; export { detectBinaryFormat } from './binary-detector.js'; export type { BinaryInfo } from './binary-detector.js'; diff --git a/packages/codelldb-common/tests/codelldb-resolver.test.ts b/packages/codelldb-common/tests/codelldb-resolver.test.ts index 28fcd08c..4574e23f 100644 --- a/packages/codelldb-common/tests/codelldb-resolver.test.ts +++ b/packages/codelldb-common/tests/codelldb-resolver.test.ts @@ -18,6 +18,7 @@ vi.mock('fs/promises', () => ({ import { resolveCodeLLDBExecutable, + resolveCodeLLDBExecutableWithSource, getCodeLLDBVersion, DEFAULT_CODELLDB_VERSION, getCodeLLDBPlatformDir, @@ -138,6 +139,79 @@ describe('codelldb-resolver', () => { }); }); + describe('resolveCodeLLDBExecutableWithSource (issue #423)', () => { + const pkgRoot = path.resolve('/npx-install/node_modules/@debugmcp/codelldb-linux-x64'); + + it('attributes a vendored candidate hit to source "vendored"', async () => { + stubPlatform('linux', 'x64'); + accessMock.mockResolvedValue(undefined); + + const result = await resolveCodeLLDBExecutableWithSource(); + + expect(result).not.toBeNull(); + expect(result!.source).toBe('vendored'); + expect(result!.path).toBe(accessMock.mock.calls[0][0]); + }); + + it('attributes a CODELLDB_PATH hit to source "env:CODELLDB_PATH"', async () => { + stubPlatform('darwin', 'arm64'); + vi.stubEnv('CODELLDB_PATH', '/custom/codelldb'); + accessMock.mockImplementation((p: string) => + p === '/custom/codelldb' ? Promise.resolve() : Promise.reject(new Error('ENOENT')) + ); + + await expect( + resolveCodeLLDBExecutableWithSource({ resolvePlatformPackageRoot: () => null }) + ).resolves.toEqual({ path: '/custom/codelldb', source: 'env:CODELLDB_PATH' }); + }); + + it('attributes a platform-package hit to source "platform-package"', async () => { + stubPlatform('linux', 'x64'); + const expected = path.join(pkgRoot, 'adapter', 'codelldb'); + accessMock.mockImplementation((p: string) => + p === expected ? Promise.resolve() : Promise.reject(new Error('ENOENT')) + ); + + await expect( + resolveCodeLLDBExecutableWithSource({ resolvePlatformPackageRoot: () => pkgRoot }) + ).resolves.toEqual({ path: expected, source: 'platform-package' }); + }); + + it('returns null when nothing resolves', async () => { + stubPlatform('linux', 'x64'); + accessMock.mockRejectedValue(new Error('ENOENT')); + + await expect( + resolveCodeLLDBExecutableWithSource({ resolvePlatformPackageRoot: () => null }) + ).resolves.toBeNull(); + }); + + it('returns null on unsupported platforms without touching the filesystem', async () => { + stubPlatform('freebsd'); + + await expect(resolveCodeLLDBExecutableWithSource()).resolves.toBeNull(); + expect(accessMock).not.toHaveBeenCalled(); + }); + + it('resolveCodeLLDBExecutable delegates: same path, same probe sequence', async () => { + stubPlatform('linux', 'x64'); + accessMock + .mockRejectedValueOnce(new Error('ENOENT')) + .mockResolvedValueOnce(undefined); + + const plain = await resolveCodeLLDBExecutable(); + + accessMock.mockReset(); + accessMock + .mockRejectedValueOnce(new Error('ENOENT')) + .mockResolvedValueOnce(undefined); + + const withSource = await resolveCodeLLDBExecutableWithSource(); + + expect(withSource!.path).toBe(plain); + }); + }); + describe('getCodeLLDBVersion', () => { it('returns null when the executable cannot be resolved', async () => { stubPlatform('win32'); diff --git a/tests/adapters/java/unit/java-utils.test.ts b/tests/adapters/java/unit/java-utils.test.ts index 67f3641a..f0fc883b 100644 --- a/tests/adapters/java/unit/java-utils.test.ts +++ b/tests/adapters/java/unit/java-utils.test.ts @@ -4,6 +4,7 @@ import { EventEmitter } from 'events'; import path from 'path'; import { findJavaExecutable, + findJavacExecutable, getJavaVersion, getJavaSearchPaths } from '@debugmcp/adapter-java'; @@ -115,6 +116,58 @@ describe('java-utils', () => { }); }); + describe('findJavacExecutable (issue #423)', () => { + const ext = process.platform === 'win32' ? '.exe' : ''; + + const validatingSpawn = (validPaths: string[]) => { + mockSpawn.mockImplementation(((cmd: string) => { + const proc = new EventEmitter() as any; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + process.nextTick(() => { + if (validPaths.includes(cmd)) { + proc.stdout.emit('data', Buffer.from('javac 21.0.6\n')); + proc.emit('exit', 0); + } else { + proc.emit('error', new Error('ENOENT')); + } + }); + return proc; + }) as any); + }; + + it('returns the javac sibling of a resolved java path', async () => { + const javaPath = path.join(path.sep, 'jdk', 'bin', `java${ext}`); + const javacPath = path.join(path.sep, 'jdk', 'bin', `javac${ext}`); + validatingSpawn([javacPath]); + + await expect(findJavacExecutable(javaPath)).resolves.toBe(javacPath); + }); + + it('falls back to JAVA_HOME/bin/javac when the sibling does not validate', async () => { + const home = path.join(path.sep, 'opt', 'jdk21'); + vi.stubEnv('JAVA_HOME', home); + const javacHome = path.join(home, 'bin', `javac${ext}`); + validatingSpawn([javacHome]); + + await expect(findJavacExecutable(path.join(path.sep, 'elsewhere', 'java'))).resolves.toBe(javacHome); + }); + + it('falls back to bare javac on PATH', async () => { + vi.stubEnv('JAVA_HOME', ''); + validatingSpawn(['javac']); + + await expect(findJavacExecutable()).resolves.toBe('javac'); + }); + + it('returns null when no javac validates anywhere', async () => { + vi.stubEnv('JAVA_HOME', ''); + validatingSpawn([]); + + await expect(findJavacExecutable(path.join(path.sep, 'jdk', 'bin', 'java'))).resolves.toBeNull(); + }); + }); + describe('getJavaVersion', () => { it('should parse standard version string from stderr', async () => { mockSpawn.mockImplementation(() => { From 59f0d3ceef97c3671b38059f94e8d0df3f921b26 Mon Sep 17 00:00:00 2001 From: JF Date: Sat, 22 Aug 2026 19:03:13 -0400 Subject: [PATCH 2/6] =?UTF-8?q?feat(cli):=20mcp-debugger=20doctor=20?= =?UTF-8?q?=E2=80=94=20environment=20self-check=20command=20(#423)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One command that answers "why is this language unavailable and how do I fix it": doctor prints adapter -> runtime (path + version) -> debug backend -> verdict + fix hint for all nine adapters, plus host-platform checks (Linux Yama ptrace_scope, container workspace mount). - doctor [languages...] [--json] [--timeout ]; exit 0/1 gates on the requested languages (overview mode always exits 0), 2 = doctor error. - Reuses the exact probing rails of list_supported_languages (one factory.validate() per language + computeModeAvailability) so doctor verdicts can never disagree with the server; where the server fails open on probe errors, doctor reports broken and records probe.failed / probe.timedOut so the divergence is visible. - Per-language timeout (default 10s), all probes in parallel; a timed-out probe forces exit after a stdout drain so a hung toolchain child cannot wedge the process. - console.* is noop'd process-wide, so output goes through process.stdout.write (check-rust-binary pattern); no janitor for doctor (issue #399 contract); handler lazy-imported (issue #400). - npx bundle picks the command up automatically via cli-entry -> main(). Co-Authored-By: Claude Fable 5 --- README.md | 2 +- src/cli/commands/doctor/diagnose.ts | 296 +++++++++++++++++ src/cli/commands/doctor/format.ts | 131 ++++++++ src/cli/commands/doctor/index.ts | 110 ++++++ src/cli/commands/doctor/platform-checks.ts | 161 +++++++++ src/cli/commands/doctor/presenters.ts | 170 ++++++++++ src/cli/setup.ts | 22 ++ src/index.ts | 13 +- tests/e2e/doctor-smoke.test.ts | 70 ++++ tests/unit/cli/doctor/diagnose.test.ts | 314 ++++++++++++++++++ tests/unit/cli/doctor/format.test.ts | 142 ++++++++ tests/unit/cli/doctor/index.test.ts | 193 +++++++++++ tests/unit/cli/doctor/platform-checks.test.ts | 127 +++++++ tests/unit/cli/doctor/presenters.test.ts | 173 ++++++++++ tests/unit/cli/setup.test.ts | 75 ++++- tests/unit/index.test.ts | 27 ++ 16 files changed, 2023 insertions(+), 3 deletions(-) create mode 100644 src/cli/commands/doctor/diagnose.ts create mode 100644 src/cli/commands/doctor/format.ts create mode 100644 src/cli/commands/doctor/index.ts create mode 100644 src/cli/commands/doctor/platform-checks.ts create mode 100644 src/cli/commands/doctor/presenters.ts create mode 100644 tests/e2e/doctor-smoke.test.ts create mode 100644 tests/unit/cli/doctor/diagnose.test.ts create mode 100644 tests/unit/cli/doctor/format.test.ts create mode 100644 tests/unit/cli/doctor/index.test.ts create mode 100644 tests/unit/cli/doctor/platform-checks.test.ts create mode 100644 tests/unit/cli/doctor/presenters.test.ts diff --git a/README.md b/README.md index 96a233fe..b71e006e 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ The server also serves condensed guidance in-band: MCP `instructions` on connect ## 🚀 Quick Start -> **Requirements:** Node.js 22+ for the server. Each language you debug also needs its own toolchain installed (Python + debugpy, Ruby + the `debug` gem / `rdbg`, Node.js, Go + Delve, JDK 21+, .NET SDK, the Rust toolchain, or a C/C++ compiler — g++/clang++, only needed for source-file launch). +> **Requirements:** Node.js 22+ for the server. Each language you debug also needs its own toolchain installed (Python + debugpy, Ruby + the `debug` gem / `rdbg`, Node.js, Go + Delve, JDK 21+, .NET SDK, the Rust toolchain, or a C/C++ compiler — g++/clang++, only needed for source-file launch). Not sure what's installed? Run `npx @debugmcp/mcp-debugger doctor` for a per-adapter toolchain report. > > **CodeLLDB platform note (npx/npm installs):** the CodeLLDB debug engine ships as per-platform optional dependencies (`@debugmcp/codelldb-win32-x64`, `-darwin-x64`, `-darwin-arm64`, `-linux-x64`, `-linux-arm64`) — npm installs exactly the one matching your platform, so Rust and C/C++ debugging work out of the box everywhere npm serves. If you install with `--omit=optional`, set `CODELLDB_PATH` to a [CodeLLDB release](https://github.com/vadimcn/codelldb/releases) binary instead, or use the Docker image. diff --git a/src/cli/commands/doctor/diagnose.ts b/src/cli/commands/doctor/diagnose.ts new file mode 100644 index 00000000..38eb0824 --- /dev/null +++ b/src/cli/commands/doctor/diagnose.ts @@ -0,0 +1,296 @@ +/** + * Orchestration for `mcp-debugger doctor` (issue #423). + * + * Reuses the exact probing surface the server uses for + * list_supported_languages — registry.listAvailableAdapters(), one + * factory.validate() per installed language, computeModeAvailability() — + * so doctor's launch/attach availability can never disagree with the + * server. Doctor adds what the server deliberately omits: per-probe + * timeouts, doctor-only extras, host-platform checks, and an honest + * verdict where the server fails open (recorded via probe.failed / + * probe.timedOut so the divergence is visible). + */ +import type { + AttachMechanism, + FactoryValidationResult, + IAdapterFactory, + IEnvironment, + IFileSystem, + ILogger +} from '@debugmcp/shared'; +import { computeModeAvailability, type LanguageModes } from '../../../utils/language-availability.js'; +import { getDisabledLanguages } from '../../../utils/language-config.js'; +import { + checkContainerWorkspace, + checkYamaPtraceScope, + type PlatformCheckResult +} from './platform-checks.js'; +import { + collectDoctorExtras, + presentLanguage, + type DoctorBackendInfo, + type DoctorRuntimeInfo +} from './presenters.js'; +import { isContainerMode } from '../../../utils/container-path-utils.js'; + +export type DoctorVerdict = 'ok' | 'warn' | 'missing' | 'disabled' | 'broken'; + +export interface LanguageDiagnosis { + language: string; + package: string; + installed: boolean; + disabled: boolean; + verdict: DoctorVerdict; + errors: string[]; + warnings: string[]; + runtime?: DoctorRuntimeInfo; + backend?: DoctorBackendInfo; + /** Verbatim computeModeAvailability output — matches list_supported_languages */ + modes?: LanguageModes; + /** Raw validate() details plus doctor-only extras */ + details?: Record; + probe: { durationMs: number; timedOut: boolean; failed: boolean }; +} + +export interface DoctorReport { + schemaVersion: 1; + version: string; + platform: { os: string; arch: string; node: string; containerMode: boolean }; + requested: string[]; + unknownLanguages: string[]; + languages: LanguageDiagnosis[]; + platformChecks: PlatformCheckResult[]; + exitCode: 0 | 1; +} + +interface RegistryAdapterEntry { + name: string; + packageName: string; + installed: boolean; + attach?: AttachMechanism; + description?: string; +} + +export interface DoctorRegistry { + listAvailableAdapters(): Promise; + getFactory(language: string): Promise; +} + +export interface DiagnoseDeps { + registry: DoctorRegistry; + environment: IEnvironment; + fileSystem: IFileSystem; + /** Disabled-language source; defaults to process.env */ + env?: NodeJS.ProcessEnv; + /** Platform for the Yama check; defaults to process.platform */ + platform?: NodeJS.Platform; + timeoutMs: number; + version: string; + logger?: ILogger; + /** Doctor-only extras collector; injectable for tests. Defaults to collectDoctorExtras. */ + collectExtras?: (language: string, details: Record) => Promise>; +} + +class ProbeTimeoutError extends Error { + constructor(timeoutMs: number) { + super(`probe timed out after ${timeoutMs}ms`); + this.name = 'ProbeTimeoutError'; + } +} + +function withTimeout(promise: Promise, timeoutMs: number): Promise { + return new Promise((resolve, reject) => { + const timer = setTimeout(() => reject(new ProbeTimeoutError(timeoutMs)), timeoutMs); + promise.then( + (value) => { + clearTimeout(timer); + resolve(value); + }, + (error) => { + clearTimeout(timer); + reject(error); + } + ); + }); +} + +const GATING_VERDICTS: ReadonlySet = new Set(['broken', 'missing', 'disabled']); + +export async function diagnose(requested: string[], deps: DiagnoseDeps): Promise { + const env = deps.env ?? process.env; + const platform = deps.platform ?? process.platform; + const collectExtras = + deps.collectExtras ?? ((language, details) => collectDoctorExtras(language, details)); + + const entries = await deps.registry.listAvailableAdapters(); + const knownNames = new Set(entries.map((entry) => entry.name)); + const requestedNormalized = requested.map((name) => name.trim().toLowerCase()).filter(Boolean); + const unknownLanguages = requestedNormalized.filter((name) => !knownNames.has(name)); + const disabledSet = getDisabledLanguages(env); + + const languages = await Promise.all( + entries.map((entry) => + diagnoseLanguage(entry, disabledSet.has(entry.name), deps, collectExtras) + ) + ); + + const platformChecks: PlatformCheckResult[] = [ + ...(await checkContainerWorkspace(deps.environment, deps.fileSystem)), + await checkYamaPtraceScope(deps.fileSystem, platform) + ]; + + const requestedBroken = languages.some( + (diagnosis) => + requestedNormalized.includes(diagnosis.language) && GATING_VERDICTS.has(diagnosis.verdict) + ); + const exitCode: 0 | 1 = + requestedNormalized.length > 0 && (unknownLanguages.length > 0 || requestedBroken) ? 1 : 0; + + return { + schemaVersion: 1, + version: deps.version, + platform: { + os: platform, + arch: process.arch, + node: process.version, + containerMode: isContainerMode(deps.environment) + }, + requested: requestedNormalized, + unknownLanguages, + languages, + platformChecks, + exitCode + }; +} + +async function diagnoseLanguage( + entry: RegistryAdapterEntry, + disabled: boolean, + deps: DiagnoseDeps, + collectExtras: (language: string, details: Record) => Promise> +): Promise { + const base = { + language: entry.name, + package: entry.packageName, + installed: entry.installed, + disabled + }; + + if (disabled || !entry.installed) { + const modes = await computeModeAvailability({ + language: entry.name, + packageName: entry.packageName, + installed: entry.installed, + disabled, + attach: entry.attach ?? 'none', + logger: deps.logger + }); + return { + ...base, + verdict: disabled ? 'disabled' : 'missing', + errors: [], + warnings: [], + modes, + probe: { durationMs: 0, timedOut: false, failed: false } + }; + } + + const factory = await deps.registry.getFactory(entry.name).catch(() => undefined); + + if (!factory || typeof factory.validate !== 'function') { + // Mirrors the server: no factory means no probe, and availability is + // assumed. Doctor surfaces that it could not actually look. + const modes = await computeModeAvailability({ + language: entry.name, + packageName: entry.packageName, + installed: true, + disabled: false, + attach: entry.attach ?? 'none', + logger: deps.logger + }); + return { + ...base, + verdict: 'warn', + errors: [], + warnings: ['Adapter factory could not be loaded, so the toolchain was not probed'], + modes, + probe: { durationMs: 0, timedOut: false, failed: false } + }; + } + + const started = Date.now(); + let validation: FactoryValidationResult | undefined; + let probeError: unknown; + let timedOut = false; + try { + validation = await withTimeout(factory.validate(), deps.timeoutMs); + } catch (error) { + probeError = error; + timedOut = error instanceof ProbeTimeoutError; + } + const durationMs = Date.now() - started; + const failed = probeError !== undefined && !timedOut; + + // Feed computeModeAvailability the same outcome the server would see: the + // memoized result, or a throwing probe so its fail-open path runs. + const modes = await computeModeAvailability({ + language: entry.name, + packageName: entry.packageName, + installed: true, + disabled: false, + attach: factory.getMetadata().modes?.attach ?? entry.attach ?? 'none', + validate: validation + ? async () => validation + : async () => { + throw probeError; + }, + logger: deps.logger + }); + + let details = validation?.details ? { ...validation.details } : undefined; + if (validation) { + try { + const extras = await withTimeout(collectExtras(entry.name, details ?? {}), deps.timeoutMs); + if (extras && Object.keys(extras).length > 0) { + details = { ...(details ?? {}), ...extras }; + } + } catch { + // Extras are best-effort; the verdict stands on validate() alone. + } + } + + let verdict: DoctorVerdict; + let errors: string[]; + let warnings: string[]; + if (!validation) { + verdict = 'broken'; + errors = [ + timedOut + ? `Toolchain probe timed out after ${deps.timeoutMs}ms (the server assumes available when a probe fails)` + : `Toolchain probe failed: ${probeError instanceof Error ? probeError.message : String(probeError)}` + ]; + warnings = []; + } else if (!validation.valid) { + verdict = 'broken'; + errors = validation.errors; + warnings = validation.warnings; + } else { + verdict = validation.warnings.length > 0 ? 'warn' : 'ok'; + errors = []; + warnings = validation.warnings; + } + + const view = presentLanguage(entry.name, details); + + return { + ...base, + verdict, + errors, + warnings, + runtime: view.runtime, + backend: view.backend, + modes, + details, + probe: { durationMs, timedOut, failed } + }; +} diff --git a/src/cli/commands/doctor/format.ts b/src/cli/commands/doctor/format.ts new file mode 100644 index 00000000..1fc1d760 --- /dev/null +++ b/src/cli/commands/doctor/format.ts @@ -0,0 +1,131 @@ +/** + * Output formatting for `mcp-debugger doctor` (issue #423). + * + * Human output is a hand-padded table (no table library in this repo — the + * house style is check-rust-binary's assembled lines with ✅/⚠️/❌ markers) + * followed by platform checks and a Fixes block. JSON output is the + * DoctorReport verbatim. + */ +import type { DoctorReport, DoctorVerdict, LanguageDiagnosis } from './diagnose.js'; +import type { PlatformCheckResult } from './platform-checks.js'; + +const VERDICT_MARKS: Record = { + ok: '✅ ok', + warn: '⚠️ warn', + missing: '❌ missing', + disabled: '🚫 disabled', + broken: '❌ broken' +}; + +const STATUS_MARKS: Record = { + ok: '✅', + warn: '⚠️', + broken: '❌', + skipped: '—' +}; + +function cellFor(info: { path?: string; version?: string; label?: string; source?: string } | undefined): string { + if (!info) { + return '—'; + } + const parts: string[] = []; + if (info.label && info.label.startsWith('(')) { + // "(built-in)" style labels stand alone + return info.label; + } + if (info.label) parts.push(info.label); + if (info.version) parts.push(info.version); + if (info.source) parts.push(`(${info.source})`); + if (info.path) parts.push(info.path); + return parts.length > 0 ? parts.join(' ') : '—'; +} + +function padColumns(rows: string[][]): string[] { + const widths: number[] = []; + for (const row of rows) { + row.forEach((cell, index) => { + widths[index] = Math.max(widths[index] ?? 0, cell.length); + }); + } + return rows.map((row) => + row + .map((cell, index) => (index === row.length - 1 ? cell : cell.padEnd(widths[index] + 2))) + .join('') + .trimEnd() + ); +} + +function needsAttention(diagnosis: LanguageDiagnosis): boolean { + return diagnosis.verdict !== 'ok'; +} + +export function formatHumanReport(report: DoctorReport): string { + const lines: string[] = []; + lines.push( + `mcp-debugger doctor ${report.version} (${report.platform.os}-${report.platform.arch}, node ${report.platform.node})` + ); + lines.push(''); + + const tableRows: string[][] = [ + ['Adapter', 'Runtime', 'Debug backend', 'Verdict'], + ...report.languages.map((diagnosis) => [ + diagnosis.language, + cellFor(diagnosis.runtime), + cellFor(diagnosis.backend), + VERDICT_MARKS[diagnosis.verdict] + ]) + ]; + lines.push(...padColumns(tableRows)); + lines.push(''); + + lines.push('Platform checks'); + for (const check of report.platformChecks) { + lines.push(` ${STATUS_MARKS[check.status]} ${check.label}: ${check.detail}`); + if (check.fixHint) { + lines.push(` fix: ${check.fixHint}`); + } + } + lines.push(''); + + if (report.unknownLanguages.length > 0) { + lines.push(`Unknown languages requested: ${report.unknownLanguages.join(', ')}`); + lines.push(''); + } + + const attention = report.languages.filter(needsAttention); + const fixLines: string[] = []; + for (const diagnosis of attention) { + const reasons = [ + ...diagnosis.errors, + ...diagnosis.warnings, + ...(diagnosis.modes?.launch.available === false && diagnosis.errors.length === 0 + ? [diagnosis.modes.launch.reason ?? ''] + : []) + ].filter((reason) => reason.length > 0); + for (const reason of reasons) { + fixLines.push(` ${diagnosis.language}: ${reason}`); + } + } + if (fixLines.length > 0) { + lines.push('Fixes'); + lines.push(...fixLines); + lines.push(''); + } + + if (attention.length === 0) { + lines.push(`All ${report.languages.length} adapters healthy.`); + } else { + lines.push( + `${attention.length} of ${report.languages.length} adapters need attention.` + + (report.requested.length === 0 + ? " Run 'mcp-debugger doctor ' to gate the exit code on a specific language." + : '') + ); + } + + return lines.join('\n'); +} + +export function formatJsonReport(report: DoctorReport): string { + return JSON.stringify(report, null, 2); +} diff --git a/src/cli/commands/doctor/index.ts b/src/cli/commands/doctor/index.ts new file mode 100644 index 00000000..50f613f8 --- /dev/null +++ b/src/cli/commands/doctor/index.ts @@ -0,0 +1,110 @@ +/** + * Handler for `mcp-debugger doctor` (issue #423). + * + * console.* is noop'd process-wide before any import runs (src/index.ts), so + * all output goes through process.stdout/stderr writes — the same pattern as + * check-rust-binary. The exit code is returned (the wiring assigns it to + * process.exitCode); process.exit is only forced when a probe timed out, + * because a hung toolchain child can otherwise keep the event loop alive + * forever. + */ +import type { IEnvironment, IFileSystem, ILogger } from '@debugmcp/shared'; +import { createProductionDependencies } from '../../../container/dependencies.js'; +import { getVersion } from '../../version.js'; +import { diagnose, type DoctorRegistry } from './diagnose.js'; +import { formatHumanReport, formatJsonReport } from './format.js'; +import type { DoctorOptions } from '../../setup.js'; + +export interface DoctorDependencies { + adapterRegistry: unknown; + environment: IEnvironment; + fileSystem: IFileSystem; + logger: ILogger; + disposeLogger?: () => void; +} + +export interface DoctorHandlerOverrides { + createDependencies?: () => DoctorDependencies; + writeOutput?: (text: string) => void; + writeError?: (text: string) => void; + /** Invoked only when a probe timed out (hung-child containment). */ + exit?: (code: number) => void; + env?: NodeJS.ProcessEnv; + platform?: NodeJS.Platform; +} + +function defaultWriteOutput(text: string): void { + process.stdout.write(text); + if (!text.endsWith('\n')) { + process.stdout.write('\n'); + } +} + +function defaultWriteError(text: string): void { + process.stderr.write(`${text}\n`); +} + +/** Let stdout flush before a forced exit (Windows pipes drop unflushed data). */ +function drainStdout(): Promise { + return new Promise((resolve) => { + process.stdout.write('', () => resolve()); + }); +} + +export async function handleDoctorCommand( + languages: string[], + options: DoctorOptions = {}, + overrides: DoctorHandlerOverrides = {} +): Promise { + const writeOutput = overrides.writeOutput ?? defaultWriteOutput; + const writeError = overrides.writeError ?? defaultWriteError; + const exit = overrides.exit ?? ((code: number) => process.exit(code)); + + const timeoutRaw = options.timeout ?? '10000'; + const timeoutMs = Number.parseInt(timeoutRaw, 10); + if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) { + writeError(`Invalid --timeout value: '${timeoutRaw}' (expected a positive number of milliseconds)`); + return 2; + } + + let deps: DoctorDependencies | undefined; + try { + const createDependencies = + overrides.createDependencies ?? (() => createProductionDependencies({ logLevel: 'error' })); + deps = createDependencies(); + + const registry = deps.adapterRegistry as DoctorRegistry; + if ( + typeof registry?.listAvailableAdapters !== 'function' || + typeof registry?.getFactory !== 'function' + ) { + throw new Error('The adapter registry does not support doctor probing'); + } + + const report = await diagnose(languages, { + registry, + environment: deps.environment, + fileSystem: deps.fileSystem, + env: overrides.env, + platform: overrides.platform, + timeoutMs, + version: getVersion(), + logger: deps.logger + }); + + writeOutput(options.json ? formatJsonReport(report) : formatHumanReport(report)); + + if (report.languages.some((diagnosis) => diagnosis.probe.timedOut)) { + // A timed-out validate may have left a hung child holding the event + // loop; report honestly, then leave. + await drainStdout(); + exit(report.exitCode); + } + return report.exitCode; + } catch (error) { + writeError(error instanceof Error ? error.message : String(error)); + return 2; + } finally { + deps?.disposeLogger?.(); + } +} diff --git a/src/cli/commands/doctor/platform-checks.ts b/src/cli/commands/doctor/platform-checks.ts new file mode 100644 index 00000000..0eaf9fdc --- /dev/null +++ b/src/cli/commands/doctor/platform-checks.ts @@ -0,0 +1,161 @@ +/** + * Host-platform checks for `mcp-debugger doctor` (issue #423). + * + * These probe host conditions no adapter owns: the Linux Yama ptrace scope + * (gates attach for the CodeLLDB-backed adapters) and the container + * workspace mount. Pure reads — nothing here mutates the system. + */ +import type { IEnvironment, IFileSystem } from '@debugmcp/shared'; +import { isContainerMode, getWorkspaceRoot } from '../../../utils/container-path-utils.js'; + +export interface PlatformCheckResult { + id: 'container-mode' | 'workspace-mount' | 'yama-ptrace-scope'; + label: string; + status: 'ok' | 'warn' | 'broken' | 'skipped'; + detail: string; + fixHint?: string; +} + +const YAMA_SYSCTL_FILE = '/proc/sys/kernel/yama/ptrace_scope'; +const YAMA_FIX_HINT = + 'sudo sysctl kernel.yama.ptrace_scope=0 (host) or --cap-add=SYS_PTRACE (containers); see docs/cpp/README.md'; + +/** + * Semantics per docs/cpp/README.md: 0 = unrestricted, 1 = ancestor-only + * (blocks attaching to arbitrary PIDs), 2 = CAP_SYS_PTRACE only, + * 3 = permanently disabled until reboot. + */ +export async function checkYamaPtraceScope( + fileSystem: IFileSystem, + platform: NodeJS.Platform +): Promise { + const base = { id: 'yama-ptrace-scope' as const, label: 'yama ptrace_scope' }; + + if (platform !== 'linux') { + return { ...base, status: 'skipped', detail: 'linux only' }; + } + + let raw: string; + try { + raw = await fileSystem.readFile(YAMA_SYSCTL_FILE, 'utf8'); + } catch { + return { ...base, status: 'skipped', detail: 'Yama LSM not present' }; + } + + const scope = Number.parseInt(raw.trim(), 10); + switch (scope) { + case 0: + return { ...base, status: 'ok', detail: 'ptrace_scope=0 (attach unrestricted)' }; + case 1: + return { + ...base, + status: 'warn', + detail: 'ptrace_scope=1 (attach limited to child processes)', + fixHint: YAMA_FIX_HINT + }; + case 2: + return { + ...base, + status: 'warn', + detail: 'ptrace_scope=2 (attach requires CAP_SYS_PTRACE)', + fixHint: YAMA_FIX_HINT + }; + case 3: + return { + ...base, + status: 'broken', + detail: 'ptrace_scope=3 (attach permanently disabled until reboot)' + }; + default: + return { ...base, status: 'skipped', detail: `unrecognized ptrace_scope value: ${raw.trim()}` }; + } +} + +/** + * Container-mode detection plus a sanity check of the workspace mount — + * `getWorkspaceRoot` only reads env vars, so the existence/emptiness of the + * mounted directory is verified here (a missing -v mount is the most common + * container-mode failure, docs/docker-support.md). + */ +export async function checkContainerWorkspace( + environment: IEnvironment, + fileSystem: IFileSystem +): Promise { + const containerBase = { id: 'container-mode' as const, label: 'container mode' }; + const mountBase = { id: 'workspace-mount' as const, label: 'workspace mount' }; + + if (!isContainerMode(environment)) { + return [ + { ...containerBase, status: 'ok', detail: 'not running in container mode' }, + { ...mountBase, status: 'skipped', detail: 'host mode' } + ]; + } + + const containerResult: PlatformCheckResult = { + ...containerBase, + status: 'ok', + detail: 'MCP_CONTAINER=true' + }; + + let root: string; + try { + root = getWorkspaceRoot(environment); + } catch { + return [ + containerResult, + { + ...mountBase, + status: 'broken', + detail: 'MCP_WORKSPACE_ROOT is not set', + fixHint: 'Set MCP_WORKSPACE_ROOT (the Docker image sets /workspace) and mount your project there' + } + ]; + } + + try { + const stats = await fileSystem.stat(root); + if (!stats.isDirectory()) { + return [ + containerResult, + { + ...mountBase, + status: 'broken', + detail: `${root} exists but is not a directory` + } + ]; + } + } catch { + return [ + containerResult, + { + ...mountBase, + status: 'broken', + detail: `${root} does not exist — is the volume mounted?`, + fixHint: 'docker run -v "$(pwd)":/workspace ... (see docs/docker-support.md)' + } + ]; + } + + try { + const entries = await fileSystem.readdir(root); + if (entries.length === 0) { + return [ + containerResult, + { + ...mountBase, + status: 'warn', + detail: `${root} is mounted but empty — wrong host directory?` + } + ]; + } + return [ + containerResult, + { ...mountBase, status: 'ok', detail: `${root} (${entries.length} entries)` } + ]; + } catch { + return [ + containerResult, + { ...mountBase, status: 'warn', detail: `${root} exists but could not be listed` } + ]; + } +} diff --git a/src/cli/commands/doctor/presenters.ts b/src/cli/commands/doctor/presenters.ts new file mode 100644 index 00000000..5a944107 --- /dev/null +++ b/src/cli/commands/doctor/presenters.ts @@ -0,0 +1,170 @@ +/** + * Per-language presentation for `mcp-debugger doctor` (issue #423). + * + * presentLanguage maps the raw factory-validate `details` (plus doctor-only + * extras) onto the table's runtime/backend columns; collectDoctorExtras runs + * the doctor-only probes that live in the adapter packages but are too + * expensive for the server's validate() path (netcoredbg/SDK versions, javac, + * compiler banner). Both are presentation-side: no probing logic lives here. + */ + +export interface DoctorRuntimeInfo { + label: string; + path?: string; + version?: string; +} + +export interface DoctorBackendInfo { + label: string; + path?: string; + version?: string; + source?: string; +} + +export interface LanguageView { + runtime?: DoctorRuntimeInfo; + backend?: DoctorBackendInfo; +} + +type Details = Record; + +const str = (details: Details | undefined, key: string): string | undefined => { + const value = details?.[key]; + return typeof value === 'string' && value.length > 0 ? value : undefined; +}; + +export function presentLanguage(language: string, details: Details | undefined): LanguageView { + switch (language) { + case 'python': + return { + runtime: { label: 'Python', path: str(details, 'pythonPath'), version: str(details, 'pythonVersion') }, + backend: { label: 'debugpy', version: str(details, 'debugpyVersion') } + }; + case 'javascript': + return { + runtime: { label: 'Node.js', version: str(details, 'nodeVersion') }, + backend: { label: 'js-debug', source: 'vendored' } + }; + case 'ruby': + return { + runtime: { label: 'Ruby', path: str(details, 'rubyPath'), version: str(details, 'rubyVersion') }, + backend: { label: 'rdbg', path: str(details, 'rdbgPath'), version: str(details, 'rdbgVersion') } + }; + case 'go': + return { + runtime: { label: 'Go', path: str(details, 'goPath'), version: str(details, 'goVersion') }, + backend: { label: 'Delve', path: str(details, 'dlvPath'), version: str(details, 'dlvVersion') } + }; + case 'java': + return { + runtime: { label: 'Java', path: str(details, 'javaPath'), version: str(details, 'javaVersion') }, + backend: { label: 'JDI bridge', path: str(details, 'jdiBridgeDir') } + }; + case 'dotnet': + return { + runtime: { label: '.NET SDK', version: str(details, 'dotnetSdkVersion') }, + backend: { + label: 'netcoredbg', + path: str(details, 'debuggerPath'), + version: str(details, 'netcoredbgVersion') + } + }; + case 'rust': + return { + runtime: { label: 'Rust', version: str(details, 'cargoVersion') }, + backend: { + label: 'CodeLLDB', + path: str(details, 'codelldbPath'), + version: str(details, 'codelldbVersion'), + source: str(details, 'codelldbSource') + } + }; + case 'cpp': { + // The --version banner already names the command; only show the bare + // command when no banner was captured. + const compilerVersion = str(details, 'compilerVersion'); + return { + runtime: { + label: 'C/C++ compiler', + path: compilerVersion ? undefined : str(details, 'compiler'), + version: compilerVersion + }, + backend: { + label: 'CodeLLDB', + path: str(details, 'codelldbPath'), + version: str(details, 'codelldbVersion'), + source: str(details, 'codelldbSource') + } + }; + } + case 'mock': + return { + runtime: { label: '(built-in)' }, + backend: { label: '(built-in)' } + }; + default: + return {}; + } +} + +type ImportModule = (id: string) => Promise; + +const defaultImportModule: ImportModule = (id) => import(/* @vite-ignore */ id); + +/** + * Doctor-only probes, run per installed language after validate(). Each is + * best-effort: an unimportable adapter package or a failing probe yields no + * extras rather than an error — the verdict already stands on validate(). + */ +export async function collectDoctorExtras( + language: string, + details: Details, + options: { importModule?: ImportModule } = {} +): Promise
{ + const importModule = options.importModule ?? defaultImportModule; + try { + switch (language) { + case 'dotnet': { + const mod = (await importModule('@debugmcp/adapter-dotnet')) as Partial<{ + getNetcoredbgVersion(path: string): Promise; + getDotnetSdkVersion(): Promise; + }>; + const extras: Details = {}; + const debuggerPath = str(details, 'debuggerPath'); + if (debuggerPath && typeof mod.getNetcoredbgVersion === 'function') { + const version = await mod.getNetcoredbgVersion(debuggerPath); + if (version) extras.netcoredbgVersion = version; + } + if (typeof mod.getDotnetSdkVersion === 'function') { + const sdk = await mod.getDotnetSdkVersion(); + if (sdk) extras.dotnetSdkVersion = sdk; + } + return extras; + } + case 'java': { + const mod = (await importModule('@debugmcp/adapter-java')) as Partial<{ + findJavacExecutable(javaPath?: string): Promise; + }>; + if (typeof mod.findJavacExecutable === 'function') { + const javacPath = await mod.findJavacExecutable(str(details, 'javaPath')); + if (javacPath) return { javacPath }; + } + return {}; + } + case 'cpp': { + const mod = (await importModule('@debugmcp/adapter-cpp')) as Partial<{ + getCompilerInfo(): Promise<{ command: string; version: string | null } | null>; + }>; + if (typeof mod.getCompilerInfo === 'function') { + const info = await mod.getCompilerInfo(); + if (info?.version) return { compilerVersion: info.version }; + } + return {}; + } + default: + return {}; + } + } catch { + return {}; + } +} diff --git a/src/cli/setup.ts b/src/cli/setup.ts index 977c92df..ab1a97c9 100644 --- a/src/cli/setup.ts +++ b/src/cli/setup.ts @@ -17,6 +17,11 @@ export interface CheckRustBinaryOptions { json?: boolean; } +export interface DoctorOptions { + json?: boolean; + timeout?: string; +} + export type StdioHandler = (options: StdioOptions, command?: Command) => Promise; export type SSEHandler = (options: SSEOptions, command?: Command) => Promise; export type HttpHandler = (options: HttpOptions, command?: Command) => Promise; @@ -25,6 +30,11 @@ export type CheckRustBinaryHandler = ( options: CheckRustBinaryOptions, command?: Command ) => Promise; +export type DoctorHandler = ( + languages: string[], + options: DoctorOptions, + command?: Command +) => Promise; export function createCLI(name: string, description: string, version: string): Command { const program = new Command(); @@ -78,6 +88,18 @@ export function setupHttpCommand(program: Command, handler: HttpHandler): void { }); } +export function setupDoctorCommand(program: Command, handler: DoctorHandler): void { + program + .command('doctor') + .description('Diagnose language toolchains and debug backends (exit code reflects the requested languages)') + .argument('[languages...]', 'Languages to check and gate the exit code on (default: report all, exit 0)') + .option('--json', 'Emit JSON output', false) + .option('--timeout ', 'Per-language probe timeout in milliseconds', '10000') + .action(async (languages: string[], options: DoctorOptions, command: Command) => { + await handler(languages, options, command); + }); +} + export function setupCheckRustBinaryCommand( program: Command, handler: CheckRustBinaryHandler diff --git a/src/index.ts b/src/index.ts index 1ec9d9ee..4bfbe8d5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -48,6 +48,7 @@ import { setupSSECommand, setupHttpCommand, setupCheckRustBinaryCommand, + setupDoctorCommand, } from './cli/setup.js'; import { handleStdioCommand } from './cli/stdio-command.js'; import { handleCheckRustBinaryCommand } from './cli/commands/check-rust-binary.js'; @@ -138,7 +139,16 @@ export async function main(): Promise { setupCheckRustBinaryCommand(program, (binaryPath, options) => handleCheckRustBinaryCommand(binaryPath, options) ); - + + // Doctor is a diagnostic, not a server: no janitor (issue #399 contract), + // lazy import so stdio startup never pays for it (issue #400 pattern), and + // the exit code lands on process.exitCode rather than a throw (main().catch + // would collapse every failure to 1). + setupDoctorCommand(program, async (languages, options) => { + const { handleDoctorCommand } = await import('./cli/commands/doctor/index.js'); + process.exitCode = await handleDoctorCommand(languages, options); + }); + // Parse command line arguments await program.parseAsync(); } @@ -194,6 +204,7 @@ export { setupSSECommand, setupHttpCommand, setupCheckRustBinaryCommand, + setupDoctorCommand, handleStdioCommand, handleCheckRustBinaryCommand }; diff --git a/tests/e2e/doctor-smoke.test.ts b/tests/e2e/doctor-smoke.test.ts new file mode 100644 index 00000000..4587d09d --- /dev/null +++ b/tests/e2e/doctor-smoke.test.ts @@ -0,0 +1,70 @@ +/** + * E2E smoke test for `mcp-debugger doctor` (issue #423). + * + * Runs the built CLI. Only mock's verdict is asserted — it needs no external + * toolchain, so this test is deterministic on every machine and CI runner. + * Real-toolchain verdicts (python, go, ...) are machine-dependent and are + * deliberately not asserted. + */ +import { describe, it, expect } from 'vitest'; +import { execFile } from 'child_process'; +import { existsSync } from 'fs'; +import * as path from 'path'; + +const projectRoot = process.cwd(); +const distEntry = path.join(projectRoot, 'dist', 'index.js'); + +interface CliResult { + code: number; + stdout: string; + stderr: string; +} + +function runCli(args: string[], timeoutMs = 90_000): Promise { + return new Promise((resolve, reject) => { + execFile( + process.execPath, + [distEntry, ...args], + { timeout: timeoutMs, windowsHide: true, maxBuffer: 10 * 1024 * 1024 }, + (error, stdout, stderr) => { + if (error && typeof error.code !== 'number') { + // Spawn failure or timeout, not a CLI exit code + reject(error); + return; + } + resolve({ code: error && typeof error.code === 'number' ? error.code : 0, stdout, stderr }); + } + ); + }); +} + +describe('doctor e2e smoke', () => { + it('emits a schemaVersion-1 JSON report covering all nine adapters, with mock ok', async () => { + if (!existsSync(distEntry)) { + throw new Error('dist/index.js not found. Run "npm run build" first.'); + } + + const result = await runCli(['doctor', 'mock', '--json']); + + expect(result.code).toBe(0); + const report = JSON.parse(result.stdout); + expect(report.schemaVersion).toBe(1); + expect(report.languages).toHaveLength(9); + const mock = report.languages.find((l: { language: string }) => l.language === 'mock'); + expect(mock.verdict).toBe('ok'); + const ids = report.platformChecks.map((c: { id: string }) => c.id); + expect(ids).toEqual(expect.arrayContaining(['container-mode', 'workspace-mount', 'yama-ptrace-scope'])); + }, 120_000); + + it('exits 1 for an unknown requested language', async () => { + if (!existsSync(distEntry)) { + throw new Error('dist/index.js not found. Run "npm run build" first.'); + } + + const result = await runCli(['doctor', 'nosuchlang', '--json']); + + expect(result.code).toBe(1); + const report = JSON.parse(result.stdout); + expect(report.unknownLanguages).toEqual(['nosuchlang']); + }, 120_000); +}); diff --git a/tests/unit/cli/doctor/diagnose.test.ts b/tests/unit/cli/doctor/diagnose.test.ts new file mode 100644 index 00000000..bc4601f5 --- /dev/null +++ b/tests/unit/cli/doctor/diagnose.test.ts @@ -0,0 +1,314 @@ +/** + * Unit tests for the doctor command's orchestration (issue #423). + * + * Everything is injected: a fake registry with fake factories, a fake + * environment/filesystem, a stubbed extras collector. No process is spawned. + */ +import { describe, it, expect, vi, afterEach } from 'vitest'; +import type { IEnvironment, IFileSystem } from '@debugmcp/shared'; +import { diagnose, type DiagnoseDeps } from '../../../../src/cli/commands/doctor/diagnose.js'; + +const makeEnvironment = (env: Record = {}): IEnvironment => ({ + get: (key: string) => env[key], + getAll: () => env, + getCurrentWorkingDirectory: () => process.cwd() +}); + +const makeFileSystem = (): IFileSystem => + ({ + readFile: vi.fn().mockRejectedValue(new Error('ENOENT')), + stat: vi.fn().mockRejectedValue(new Error('ENOENT')), + readdir: vi.fn().mockRejectedValue(new Error('ENOENT')) + }) as unknown as IFileSystem; + +interface FakeAdapterSpec { + name: string; + installed?: boolean; + attach?: 'none' | 'direct-connect' | 'spawn'; + validate?: () => Promise<{ valid: boolean; errors: string[]; warnings: string[]; details?: Record }>; +} + +function makeDeps(adapters: FakeAdapterSpec[], overrides: Partial = {}): DiagnoseDeps { + const registry = { + listAvailableAdapters: vi.fn().mockResolvedValue( + adapters.map((a) => ({ + name: a.name, + packageName: `@debugmcp/adapter-${a.name}`, + installed: a.installed ?? true, + attach: a.attach ?? 'none' + })) + ), + getFactory: vi.fn(async (language: string) => { + const spec = adapters.find((a) => a.name === language); + if (!spec || !(spec.installed ?? true) || !spec.validate) { + return undefined; + } + return { + validate: spec.validate, + getMetadata: () => ({ modes: { launch: true, attach: spec.attach ?? 'none' } }), + createAdapter: () => { + throw new Error('doctor must never instantiate adapters'); + } + }; + }) + }; + + return { + registry: registry as unknown as DiagnoseDeps['registry'], + environment: makeEnvironment(), + fileSystem: makeFileSystem(), + env: {}, + platform: 'win32', + timeoutMs: 5000, + version: '0.0.0-test', + collectExtras: async () => ({}), + ...overrides + }; +} + +const okValidate = (details: Record = {}) => async () => ({ + valid: true, + errors: [], + warnings: [], + details +}); + +afterEach(() => { + vi.useRealTimers(); +}); + +describe('diagnose', () => { + it('reports ok for a healthy adapter and carries validate details through', async () => { + const deps = makeDeps([ + { name: 'python', validate: okValidate({ pythonPath: '/usr/bin/python3', pythonVersion: '3.12.1' }) } + ]); + + const report = await diagnose([], deps); + + expect(report.schemaVersion).toBe(1); + expect(report.languages).toHaveLength(1); + const python = report.languages[0]; + expect(python.verdict).toBe('ok'); + expect(python.errors).toEqual([]); + expect(python.details).toMatchObject({ pythonPath: '/usr/bin/python3' }); + expect(python.probe.timedOut).toBe(false); + expect(python.probe.failed).toBe(false); + expect(python.modes?.launch.available).toBe(true); + }); + + it('reports warn when validation succeeds with warnings', async () => { + const deps = makeDeps([ + { + name: 'rust', + validate: async () => ({ valid: true, errors: [], warnings: ['MSVC toolchain detected'], details: {} }) + } + ]); + + const report = await diagnose([], deps); + + expect(report.languages[0].verdict).toBe('warn'); + expect(report.languages[0].warnings).toEqual(['MSVC toolchain detected']); + }); + + it('reports broken with the validation errors when the toolchain is invalid', async () => { + const deps = makeDeps([ + { + name: 'go', + validate: async () => ({ valid: false, errors: ['Delve not found. Run: go install ...'], warnings: [], details: {} }) + } + ]); + + const report = await diagnose([], deps); + + expect(report.languages[0].verdict).toBe('broken'); + expect(report.languages[0].errors[0]).toContain('Delve not found'); + expect(report.languages[0].modes?.launch.available).toBe(false); + }); + + it('reports missing for adapters that are not installed', async () => { + const deps = makeDeps([{ name: 'ruby', installed: false }]); + + const report = await diagnose([], deps); + + expect(report.languages[0].verdict).toBe('missing'); + expect(report.languages[0].modes?.launch.available).toBe(false); + expect(report.languages[0].modes?.launch.reason).toContain('@debugmcp/adapter-ruby'); + }); + + it('reports disabled for adapters disabled via DEBUG_MCP_DISABLE_LANGUAGES', async () => { + const deps = makeDeps([{ name: 'python', validate: okValidate() }], { + env: { DEBUG_MCP_DISABLE_LANGUAGES: 'python' } + }); + + const report = await diagnose([], deps); + + expect(report.languages[0].verdict).toBe('disabled'); + }); + + it('reports broken with probe.failed when validate throws, while modes fail open like the server', async () => { + const deps = makeDeps([ + { + name: 'java', + validate: async () => { + throw new Error('probe exploded'); + } + } + ]); + + const report = await diagnose([], deps); + + const java = report.languages[0]; + expect(java.verdict).toBe('broken'); + expect(java.probe.failed).toBe(true); + expect(java.errors[0]).toContain('probe exploded'); + // Parity: computeModeAvailability fails open on probe errors, and doctor + // must report the same modes the server would. + expect(java.modes?.launch.available).toBe(true); + }); + + it('reports broken with probe.timedOut when validate never settles', async () => { + vi.useFakeTimers(); + const deps = makeDeps( + [{ name: 'dotnet', validate: () => new Promise(() => undefined) }], + { timeoutMs: 1000 } + ); + + const reportPromise = diagnose([], deps); + await vi.advanceTimersByTimeAsync(1100); + const report = await reportPromise; + + const dotnet = report.languages[0]; + expect(dotnet.verdict).toBe('broken'); + expect(dotnet.probe.timedOut).toBe(true); + expect(dotnet.modes?.launch.available).toBe(true); // fail-open parity + }); + + it('merges collectExtras output into the reported details', async () => { + const deps = makeDeps( + [{ name: 'dotnet', validate: okValidate({ debuggerPath: '/opt/netcoredbg' }) }], + { + collectExtras: async (language, details) => { + expect(language).toBe('dotnet'); + expect(details).toMatchObject({ debuggerPath: '/opt/netcoredbg' }); + return { netcoredbgVersion: '3.1.2-1054', dotnetSdkVersion: '8.0.301' }; + } + } + ); + + const report = await diagnose([], deps); + + expect(report.languages[0].details).toMatchObject({ + debuggerPath: '/opt/netcoredbg', + netcoredbgVersion: '3.1.2-1054', + dotnetSdkVersion: '8.0.301' + }); + }); + + it('keeps the verdict when collectExtras itself fails', async () => { + const deps = makeDeps([{ name: 'cpp', validate: okValidate() }], { + collectExtras: async () => { + throw new Error('extras exploded'); + } + }); + + const report = await diagnose([], deps); + + expect(report.languages[0].verdict).toBe('ok'); + }); + + it('lists unknown requested languages and fails the run', async () => { + const deps = makeDeps([{ name: 'python', validate: okValidate() }]); + + const report = await diagnose(['python', 'nosuchlang'], deps); + + expect(report.unknownLanguages).toEqual(['nosuchlang']); + expect(report.exitCode).toBe(1); + }); + + describe('exit code', () => { + it('is 0 in overview mode even when adapters are broken', async () => { + const deps = makeDeps([ + { name: 'go', validate: async () => ({ valid: false, errors: ['nope'], warnings: [] }) } + ]); + + const report = await diagnose([], deps); + + expect(report.exitCode).toBe(0); + }); + + it('is 0 when every requested language is ok or warn', async () => { + const deps = makeDeps([ + { name: 'python', validate: okValidate() }, + { name: 'rust', validate: async () => ({ valid: true, errors: [], warnings: ['w'] }) }, + { name: 'go', validate: async () => ({ valid: false, errors: ['nope'], warnings: [] }) } + ]); + + const report = await diagnose(['python', 'rust'], deps); + + expect(report.exitCode).toBe(0); + }); + + it('is 1 when a requested language is broken', async () => { + const deps = makeDeps([ + { name: 'python', validate: okValidate() }, + { name: 'go', validate: async () => ({ valid: false, errors: ['nope'], warnings: [] }) } + ]); + + const report = await diagnose(['go'], deps); + + expect(report.exitCode).toBe(1); + }); + + it('is 1 when a requested language is missing or disabled', async () => { + const deps = makeDeps([{ name: 'ruby', installed: false }, { name: 'python', validate: okValidate() }], { + env: { DEBUG_MCP_DISABLE_LANGUAGES: 'python' } + }); + + await expect(diagnose(['ruby'], deps)).resolves.toMatchObject({ exitCode: 1 }); + await expect(diagnose(['python'], deps)).resolves.toMatchObject({ exitCode: 1 }); + }); + + it('normalizes requested language casing', async () => { + const deps = makeDeps([{ name: 'python', validate: okValidate() }]); + + const report = await diagnose(['PYTHON'], deps); + + expect(report.unknownLanguages).toEqual([]); + expect(report.exitCode).toBe(0); + }); + }); + + it('runs the language probes in parallel', async () => { + let inFlight = 0; + let maxInFlight = 0; + const slowValidate = () => async () => { + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + await new Promise((resolve) => setTimeout(resolve, 20)); + inFlight -= 1; + return { valid: true, errors: [], warnings: [] }; + }; + const deps = makeDeps([ + { name: 'python', validate: slowValidate() }, + { name: 'go', validate: slowValidate() }, + { name: 'ruby', validate: slowValidate() } + ]); + + await diagnose([], deps); + + expect(maxInFlight).toBeGreaterThan(1); + }); + + it('includes platform checks and platform info in the report', async () => { + const deps = makeDeps([{ name: 'mock', validate: okValidate() }]); + + const report = await diagnose([], deps); + + expect(report.platform.os).toBe('win32'); + expect(report.platform.containerMode).toBe(false); + const ids = report.platformChecks.map((c) => c.id); + expect(ids).toContain('container-mode'); + expect(ids).toContain('workspace-mount'); + expect(ids).toContain('yama-ptrace-scope'); + }); +}); diff --git a/tests/unit/cli/doctor/format.test.ts b/tests/unit/cli/doctor/format.test.ts new file mode 100644 index 00000000..21a36fa7 --- /dev/null +++ b/tests/unit/cli/doctor/format.test.ts @@ -0,0 +1,142 @@ +/** + * Unit tests for the doctor command's output formatting (issue #423). + */ +import { describe, it, expect } from 'vitest'; +import { formatHumanReport, formatJsonReport } from '../../../../src/cli/commands/doctor/format.js'; +import type { DoctorReport, LanguageDiagnosis } from '../../../../src/cli/commands/doctor/diagnose.js'; + +const language = (overrides: Partial): LanguageDiagnosis => ({ + language: 'python', + package: '@debugmcp/adapter-python', + installed: true, + disabled: false, + verdict: 'ok', + errors: [], + warnings: [], + probe: { durationMs: 10, timedOut: false, failed: false }, + ...overrides +}); + +const report = (overrides: Partial = {}): DoctorReport => ({ + schemaVersion: 1, + version: '0.1.0-test', + platform: { os: 'win32', arch: 'x64', node: 'v22.0.0', containerMode: false }, + requested: [], + unknownLanguages: [], + languages: [ + language({ + language: 'python', + runtime: { label: 'Python', path: 'C:\\Python313\\python.exe', version: '3.13.2' }, + backend: { label: 'debugpy', version: '1.8.14' } + }), + language({ + language: 'go', + package: '@debugmcp/adapter-go', + verdict: 'broken', + errors: ['Delve not found. Run: go install github.com/go-delve/delve/cmd/dlv@latest'], + runtime: { label: 'Go', version: '1.24.1' } + }) + ], + platformChecks: [ + { id: 'container-mode', label: 'container mode', status: 'ok', detail: 'not running in container mode' }, + { id: 'workspace-mount', label: 'workspace mount', status: 'skipped', detail: 'host mode' }, + { id: 'yama-ptrace-scope', label: 'yama ptrace_scope', status: 'skipped', detail: 'linux only' } + ], + exitCode: 0, + ...overrides +}); + +describe('formatHumanReport', () => { + it('renders a header line, a column header, and one row per adapter', () => { + const output = formatHumanReport(report()); + const lines = output.split('\n'); + + expect(lines[0]).toContain('mcp-debugger doctor 0.1.0-test'); + expect(lines[0]).toContain('win32-x64'); + const headerLine = lines.find((l) => l.includes('Adapter') && l.includes('Verdict')); + expect(headerLine).toBeDefined(); + expect(headerLine).toContain('Runtime'); + expect(headerLine).toContain('Debug backend'); + expect(output).toContain('python'); + expect(output).toContain('3.13.2'); + expect(output).toContain('debugpy 1.8.14'); + }); + + it('aligns the verdict column across all adapter rows', () => { + const output = formatHumanReport(report()); + const lines = output.split('\n'); + const rows = lines.filter((l) => /^(python|go)\s/.test(l)); + + expect(rows).toHaveLength(2); + const verdictIndices = rows.map((row) => Math.max(row.indexOf('✅'), row.indexOf('❌'))); + expect(new Set(verdictIndices).size).toBe(1); + }); + + it('marks verdicts with the house emoji', () => { + const output = formatHumanReport(report()); + + expect(output).toContain('✅ ok'); + expect(output).toContain('❌ broken'); + }); + + it('lists fixes only for adapters that need attention', () => { + const output = formatHumanReport(report()); + + expect(output).toContain('Fixes'); + expect(output).toContain('go install github.com/go-delve/delve/cmd/dlv@latest'); + const fixesBlock = output.slice(output.indexOf('Fixes')); + expect(fixesBlock).not.toContain('python:'); + }); + + it('omits the fixes section when everything is healthy', () => { + const healthy = report({ + languages: [language({ runtime: { label: 'Python', version: '3.13.2' } })] + }); + + expect(formatHumanReport(healthy)).not.toContain('Fixes'); + }); + + it('renders the platform checks with their details', () => { + const output = formatHumanReport(report()); + + expect(output).toContain('Platform checks'); + expect(output).toContain('container mode'); + expect(output).toContain('linux only'); + }); + + it('renders platform check fix hints', () => { + const withHint = report({ + platformChecks: [ + { + id: 'yama-ptrace-scope', + label: 'yama ptrace_scope', + status: 'warn', + detail: 'ptrace_scope=1 (attach limited to child processes)', + fixHint: 'sudo sysctl kernel.yama.ptrace_scope=0' + } + ] + }); + + expect(formatHumanReport(withHint)).toContain('sudo sysctl kernel.yama.ptrace_scope=0'); + }); + + it('calls out unknown requested languages', () => { + const output = formatHumanReport(report({ unknownLanguages: ['nosuchlang'], exitCode: 1 })); + + expect(output).toContain('nosuchlang'); + }); + + it('summarizes how many adapters need attention', () => { + const output = formatHumanReport(report()); + + expect(output).toContain('1 of 2 adapters need attention'); + }); +}); + +describe('formatJsonReport', () => { + it('round-trips the report object', () => { + const input = report(); + + expect(JSON.parse(formatJsonReport(input))).toEqual(JSON.parse(JSON.stringify(input))); + }); +}); diff --git a/tests/unit/cli/doctor/index.test.ts b/tests/unit/cli/doctor/index.test.ts new file mode 100644 index 00000000..38b6771e --- /dev/null +++ b/tests/unit/cli/doctor/index.test.ts @@ -0,0 +1,193 @@ +/** + * Unit tests for the doctor command handler (issue #423). + * + * Follows the check-rust-binary handler test pattern: stub the stdout/stderr + * writers, inject fake dependencies, assert on the joined output and the + * returned exit code. Nothing real is probed. + */ +import { describe, it, expect, vi } from 'vitest'; +import { handleDoctorCommand, type DoctorDependencies } from '../../../../src/cli/commands/doctor/index.js'; + +interface FakeAdapterSpec { + name: string; + installed?: boolean; + validate?: () => Promise<{ valid: boolean; errors: string[]; warnings: string[]; details?: Record }>; +} + +function makeFakeDependencies(adapters: FakeAdapterSpec[]): DoctorDependencies & { disposeLogger: ReturnType } { + const registry = { + listAvailableAdapters: vi.fn().mockResolvedValue( + adapters.map((a) => ({ + name: a.name, + packageName: `@debugmcp/adapter-${a.name}`, + installed: a.installed ?? true, + attach: 'none' as const + })) + ), + getFactory: vi.fn(async (language: string) => { + const spec = adapters.find((a) => a.name === language); + if (!spec || !spec.validate) return undefined; + return { + validate: spec.validate, + getMetadata: () => ({ modes: { launch: true, attach: 'none' as const } }) + }; + }) + }; + return { + adapterRegistry: registry, + environment: { + get: () => undefined, + getAll: () => ({}), + getCurrentWorkingDirectory: () => process.cwd() + }, + fileSystem: { + readFile: vi.fn().mockRejectedValue(new Error('ENOENT')), + stat: vi.fn().mockRejectedValue(new Error('ENOENT')), + readdir: vi.fn().mockRejectedValue(new Error('ENOENT')) + }, + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + disposeLogger: vi.fn() + } as unknown as DoctorDependencies & { disposeLogger: ReturnType }; +} + +const okAdapter = (name: string): FakeAdapterSpec => ({ + name, + validate: async () => ({ valid: true, errors: [], warnings: [], details: {} }) +}); + +const brokenAdapter = (name: string): FakeAdapterSpec => ({ + name, + validate: async () => ({ valid: false, errors: [`${name} toolchain missing`], warnings: [] }) +}); + +function collectOutput() { + const chunks: string[] = []; + return { + writeOutput: (text: string) => chunks.push(text), + joined: () => chunks.join('\n') + }; +} + +describe('handleDoctorCommand', () => { + it('writes a human-readable report and returns 0 for a healthy overview', async () => { + const deps = makeFakeDependencies([okAdapter('python'), okAdapter('mock')]); + const output = collectOutput(); + + const code = await handleDoctorCommand([], {}, { + createDependencies: () => deps, + writeOutput: output.writeOutput, + writeError: vi.fn(), + exit: vi.fn() + }); + + expect(code).toBe(0); + expect(output.joined()).toContain('Adapter'); + expect(output.joined()).toContain('python'); + expect(deps.disposeLogger).toHaveBeenCalled(); + }); + + it('emits parseable JSON and nothing else with --json', async () => { + const deps = makeFakeDependencies([okAdapter('python')]); + const output = collectOutput(); + + const code = await handleDoctorCommand([], { json: true }, { + createDependencies: () => deps, + writeOutput: output.writeOutput, + writeError: vi.fn(), + exit: vi.fn() + }); + + expect(code).toBe(0); + const parsed = JSON.parse(output.joined()); + expect(parsed.schemaVersion).toBe(1); + expect(parsed.languages).toHaveLength(1); + }); + + it('returns 1 when a requested language is broken', async () => { + const deps = makeFakeDependencies([okAdapter('python'), brokenAdapter('go')]); + const output = collectOutput(); + + const code = await handleDoctorCommand(['go'], {}, { + createDependencies: () => deps, + writeOutput: output.writeOutput, + writeError: vi.fn(), + exit: vi.fn() + }); + + expect(code).toBe(1); + }); + + it('returns 2 and reports the error when dependency construction fails', async () => { + const writeError = vi.fn(); + + const code = await handleDoctorCommand([], {}, { + createDependencies: () => { + throw new Error('container exploded'); + }, + writeOutput: vi.fn(), + writeError, + exit: vi.fn() + }); + + expect(code).toBe(2); + expect(writeError.mock.calls.join('\n')).toContain('container exploded'); + }); + + it('disposes the logger even when diagnosis fails, and returns 2', async () => { + const deps = makeFakeDependencies([]); + (deps.adapterRegistry as unknown as { listAvailableAdapters: ReturnType }).listAvailableAdapters = + vi.fn().mockRejectedValue(new Error('registry exploded')); + const writeError = vi.fn(); + + const code = await handleDoctorCommand([], {}, { + createDependencies: () => deps, + writeOutput: vi.fn(), + writeError, + exit: vi.fn() + }); + + expect(code).toBe(2); + expect(deps.disposeLogger).toHaveBeenCalled(); + }); + + it('returns 2 for an unparseable --timeout', async () => { + const writeError = vi.fn(); + + const code = await handleDoctorCommand([], { timeout: 'soon' }, { + createDependencies: () => makeFakeDependencies([]), + writeOutput: vi.fn(), + writeError, + exit: vi.fn() + }); + + expect(code).toBe(2); + expect(writeError.mock.calls.join('\n')).toContain('timeout'); + }); + + it('force-exits only when a probe timed out (hung child containment)', async () => { + const hungDeps = makeFakeDependencies([ + { name: 'python', validate: () => new Promise(() => undefined) } + ]); + const exit = vi.fn(); + + const code = await handleDoctorCommand([], { timeout: '50' }, { + createDependencies: () => hungDeps, + writeOutput: vi.fn(), + writeError: vi.fn(), + exit + }); + + expect(exit).toHaveBeenCalledWith(code); + + const healthyDeps = makeFakeDependencies([okAdapter('python')]); + const noExit = vi.fn(); + await handleDoctorCommand([], {}, { + createDependencies: () => healthyDeps, + writeOutput: vi.fn(), + writeError: vi.fn(), + exit: noExit + }); + + expect(noExit).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/cli/doctor/platform-checks.test.ts b/tests/unit/cli/doctor/platform-checks.test.ts new file mode 100644 index 00000000..b51f533d --- /dev/null +++ b/tests/unit/cli/doctor/platform-checks.test.ts @@ -0,0 +1,127 @@ +/** + * Unit tests for the doctor command's host-platform checks (issue #423). + * All filesystem and environment access goes through injected fakes. + */ +import { describe, it, expect, vi } from 'vitest'; +import type { IEnvironment, IFileSystem } from '@debugmcp/shared'; +import { + checkYamaPtraceScope, + checkContainerWorkspace +} from '../../../../src/cli/commands/doctor/platform-checks.js'; + +const makeFileSystem = (overrides: Partial = {}): IFileSystem => + ({ + readFile: vi.fn().mockRejectedValue(new Error('ENOENT')), + stat: vi.fn().mockRejectedValue(new Error('ENOENT')), + readdir: vi.fn().mockRejectedValue(new Error('ENOENT')), + ...overrides + }) as unknown as IFileSystem; + +const makeEnvironment = (env: Record): IEnvironment => ({ + get: (key: string) => env[key], + getAll: () => env, + getCurrentWorkingDirectory: () => process.cwd() +}); + +describe('checkYamaPtraceScope', () => { + it('is skipped on non-linux platforms without touching the filesystem', async () => { + const fileSystem = makeFileSystem(); + + const result = await checkYamaPtraceScope(fileSystem, 'win32'); + + expect(result.id).toBe('yama-ptrace-scope'); + expect(result.status).toBe('skipped'); + expect(fileSystem.readFile).not.toHaveBeenCalled(); + }); + + it.each([ + [0, 'ok'], + [1, 'warn'], + [2, 'warn'], + [3, 'broken'] + ] as const)('maps ptrace_scope=%i to %s on linux', async (value, status) => { + const fileSystem = makeFileSystem({ + readFile: vi.fn().mockResolvedValue(`${value}\n`) + }); + + const result = await checkYamaPtraceScope(fileSystem, 'linux'); + + expect(result.status).toBe(status); + expect(result.detail).toContain(`ptrace_scope=${value}`); + }); + + it('offers a sysctl fix hint for restrictive scopes', async () => { + const fileSystem = makeFileSystem({ readFile: vi.fn().mockResolvedValue('1') }); + + const result = await checkYamaPtraceScope(fileSystem, 'linux'); + + expect(result.fixHint).toContain('kernel.yama.ptrace_scope'); + }); + + it('is skipped when the Yama sysctl file is unreadable (Yama absent)', async () => { + const fileSystem = makeFileSystem(); + + const result = await checkYamaPtraceScope(fileSystem, 'linux'); + + expect(result.status).toBe('skipped'); + }); +}); + +describe('checkContainerWorkspace', () => { + it('reports host mode and skips the mount check outside a container', async () => { + const environment = makeEnvironment({}); + const fileSystem = makeFileSystem(); + + const [containerMode, mount] = await checkContainerWorkspace(environment, fileSystem); + + expect(containerMode.id).toBe('container-mode'); + expect(containerMode.status).toBe('ok'); + expect(mount.id).toBe('workspace-mount'); + expect(mount.status).toBe('skipped'); + }); + + it('reports broken when MCP_WORKSPACE_ROOT is unset in container mode', async () => { + const environment = makeEnvironment({ MCP_CONTAINER: 'true' }); + const fileSystem = makeFileSystem(); + + const [, mount] = await checkContainerWorkspace(environment, fileSystem); + + expect(mount.status).toBe('broken'); + expect(mount.fixHint).toContain('MCP_WORKSPACE_ROOT'); + }); + + it('reports broken when the workspace root does not exist', async () => { + const environment = makeEnvironment({ MCP_CONTAINER: 'true', MCP_WORKSPACE_ROOT: '/workspace' }); + const fileSystem = makeFileSystem(); + + const [, mount] = await checkContainerWorkspace(environment, fileSystem); + + expect(mount.status).toBe('broken'); + expect(mount.detail).toContain('/workspace'); + }); + + it('warns when the workspace root is an empty directory', async () => { + const environment = makeEnvironment({ MCP_CONTAINER: 'true', MCP_WORKSPACE_ROOT: '/workspace' }); + const fileSystem = makeFileSystem({ + stat: vi.fn().mockResolvedValue({ isDirectory: () => true }), + readdir: vi.fn().mockResolvedValue([]) + }); + + const [, mount] = await checkContainerWorkspace(environment, fileSystem); + + expect(mount.status).toBe('warn'); + expect(mount.detail).toContain('empty'); + }); + + it('reports ok for a populated workspace mount', async () => { + const environment = makeEnvironment({ MCP_CONTAINER: 'true', MCP_WORKSPACE_ROOT: '/workspace' }); + const fileSystem = makeFileSystem({ + stat: vi.fn().mockResolvedValue({ isDirectory: () => true }), + readdir: vi.fn().mockResolvedValue(['src', 'package.json']) + }); + + const [, mount] = await checkContainerWorkspace(environment, fileSystem); + + expect(mount.status).toBe('ok'); + }); +}); diff --git a/tests/unit/cli/doctor/presenters.test.ts b/tests/unit/cli/doctor/presenters.test.ts new file mode 100644 index 00000000..601939ff --- /dev/null +++ b/tests/unit/cli/doctor/presenters.test.ts @@ -0,0 +1,173 @@ +/** + * Unit tests for the doctor command's per-language presentation mapping and + * the doctor-only extras collector (issue #423). Module loading is injected — + * no adapter package is really imported and nothing is spawned. + */ +import { describe, it, expect, vi } from 'vitest'; +import { presentLanguage, collectDoctorExtras } from '../../../../src/cli/commands/doctor/presenters.js'; + +describe('presentLanguage', () => { + it('maps python details to runtime + debugpy backend columns', () => { + const view = presentLanguage('python', { + pythonPath: 'C:\\Python313\\python.exe', + pythonVersion: '3.13.2', + debugpyVersion: '1.8.14' + }); + + expect(view.runtime).toMatchObject({ label: 'Python', path: 'C:\\Python313\\python.exe', version: '3.13.2' }); + expect(view.backend).toMatchObject({ label: 'debugpy', version: '1.8.14' }); + }); + + it('maps ruby details to ruby + rdbg columns', () => { + const view = presentLanguage('ruby', { + rubyPath: '/usr/bin/ruby', + rubyVersion: '3.4.2', + rdbgPath: '/usr/bin/rdbg', + rdbgVersion: '1.11.0' + }); + + expect(view.runtime).toMatchObject({ label: 'Ruby', version: '3.4.2' }); + expect(view.backend).toMatchObject({ label: 'rdbg', path: '/usr/bin/rdbg', version: '1.11.0' }); + }); + + it('maps go details to go + delve columns', () => { + const view = presentLanguage('go', { + goPath: '/usr/local/go/bin/go', + goVersion: '1.24.1', + dlvPath: '/home/u/go/bin/dlv', + dlvVersion: '1.26.3' + }); + + expect(view.runtime).toMatchObject({ label: 'Go', version: '1.24.1' }); + expect(view.backend).toMatchObject({ label: 'Delve', version: '1.26.3' }); + }); + + it('maps rust details including the CodeLLDB source attribution', () => { + const view = presentLanguage('rust', { + cargoVersion: 'cargo 1.85.0', + codelldbPath: '/vendor/codelldb', + codelldbVersion: '1.11.8', + codelldbSource: 'vendored' + }); + + expect(view.runtime).toMatchObject({ label: 'Rust' }); + expect(view.backend).toMatchObject({ label: 'CodeLLDB', version: '1.11.8', source: 'vendored' }); + }); + + it('maps dotnet extras into SDK runtime and netcoredbg backend columns', () => { + const view = presentLanguage('dotnet', { + debuggerPath: '/opt/netcoredbg/netcoredbg', + netcoredbgVersion: '3.1.2-1054', + dotnetSdkVersion: '8.0.301' + }); + + expect(view.runtime).toMatchObject({ label: '.NET SDK', version: '8.0.301' }); + expect(view.backend).toMatchObject({ label: 'netcoredbg', path: '/opt/netcoredbg/netcoredbg', version: '3.1.2-1054' }); + }); + + it('maps java details to java + JDI bridge columns', () => { + const view = presentLanguage('java', { + javaPath: '/opt/jdk/bin/java', + javaVersion: '21.0.6', + jdiBridgeDir: '/opt/bridge' + }); + + expect(view.runtime).toMatchObject({ label: 'Java', version: '21.0.6' }); + expect(view.backend).toMatchObject({ label: 'JDI bridge', path: '/opt/bridge' }); + }); + + it('shows only the cpp compiler banner when a version was captured (the banner names the command)', () => { + const view = presentLanguage('cpp', { + compiler: 'g++', + compilerVersion: 'g++ (MinGW-w64) 13.2.0' + }); + + expect(view.runtime).toMatchObject({ label: 'C/C++ compiler', version: 'g++ (MinGW-w64) 13.2.0' }); + expect(view.runtime?.path).toBeUndefined(); + }); + + it('falls back to the bare cpp compiler command when no version banner was captured', () => { + const view = presentLanguage('cpp', { compiler: 'g++' }); + + expect(view.runtime).toMatchObject({ label: 'C/C++ compiler', path: 'g++' }); + }); + + it('marks mock as built-in', () => { + const view = presentLanguage('mock', {}); + + expect(view.runtime?.label).toContain('built-in'); + expect(view.backend?.label).toContain('built-in'); + }); + + it('tolerates missing details entirely', () => { + const view = presentLanguage('python', undefined); + + expect(view.runtime).toMatchObject({ label: 'Python' }); + expect(view.runtime?.version).toBeUndefined(); + }); +}); + +describe('collectDoctorExtras', () => { + it('collects netcoredbg and SDK versions for dotnet via the adapter package', async () => { + const importModule = vi.fn().mockResolvedValue({ + getNetcoredbgVersion: vi.fn().mockResolvedValue('3.1.2-1054'), + getDotnetSdkVersion: vi.fn().mockResolvedValue('8.0.301') + }); + + const extras = await collectDoctorExtras('dotnet', { debuggerPath: '/opt/netcoredbg' }, { importModule }); + + expect(importModule).toHaveBeenCalledWith('@debugmcp/adapter-dotnet'); + expect(extras).toEqual({ netcoredbgVersion: '3.1.2-1054', dotnetSdkVersion: '8.0.301' }); + }); + + it('skips the netcoredbg version probe when no debugger path was resolved', async () => { + const getNetcoredbgVersion = vi.fn(); + const importModule = vi.fn().mockResolvedValue({ + getNetcoredbgVersion, + getDotnetSdkVersion: vi.fn().mockResolvedValue(null) + }); + + const extras = await collectDoctorExtras('dotnet', {}, { importModule }); + + expect(getNetcoredbgVersion).not.toHaveBeenCalled(); + expect(extras).toEqual({}); + }); + + it('collects the javac path for java', async () => { + const findJavacExecutable = vi.fn().mockResolvedValue('/opt/jdk/bin/javac'); + const importModule = vi.fn().mockResolvedValue({ findJavacExecutable }); + + const extras = await collectDoctorExtras('java', { javaPath: '/opt/jdk/bin/java' }, { importModule }); + + expect(importModule).toHaveBeenCalledWith('@debugmcp/adapter-java'); + expect(findJavacExecutable).toHaveBeenCalledWith('/opt/jdk/bin/java'); + expect(extras).toEqual({ javacPath: '/opt/jdk/bin/javac' }); + }); + + it('collects the compiler version banner for cpp', async () => { + const importModule = vi.fn().mockResolvedValue({ + getCompilerInfo: vi.fn().mockResolvedValue({ command: 'g++', version: 'g++ (MinGW-w64) 13.2.0' }) + }); + + const extras = await collectDoctorExtras('cpp', { compiler: 'g++' }, { importModule }); + + expect(extras).toEqual({ compilerVersion: 'g++ (MinGW-w64) 13.2.0' }); + }); + + it('returns no extras for languages without doctor-only probes', async () => { + const importModule = vi.fn(); + + const extras = await collectDoctorExtras('python', { pythonPath: '/usr/bin/python3' }, { importModule }); + + expect(importModule).not.toHaveBeenCalled(); + expect(extras).toEqual({}); + }); + + it('returns no extras when the adapter package cannot be imported', async () => { + const importModule = vi.fn().mockRejectedValue(new Error('MODULE_NOT_FOUND')); + + const extras = await collectDoctorExtras('dotnet', { debuggerPath: '/x' }, { importModule }); + + expect(extras).toEqual({}); + }); +}); diff --git a/tests/unit/cli/setup.test.ts b/tests/unit/cli/setup.test.ts index f9c8d191..5b56b874 100644 --- a/tests/unit/cli/setup.test.ts +++ b/tests/unit/cli/setup.test.ts @@ -1,6 +1,12 @@ import { describe, it, expect, vi } from 'vitest'; import { Command } from 'commander'; -import { createCLI, setupStdioCommand, setupSSECommand, setupHttpCommand } from '../../../src/cli/setup.js'; +import { + createCLI, + setupStdioCommand, + setupSSECommand, + setupHttpCommand, + setupDoctorCommand +} from '../../../src/cli/setup.js'; describe('CLI Setup', () => { describe('createCLI', () => { @@ -193,6 +199,59 @@ describe('CLI Setup', () => { }); }); + describe('setupDoctorCommand', () => { + it('should configure doctor command with variadic languages and options', () => { + const program = new Command(); + const mockHandler = vi.fn(); + + setupDoctorCommand(program, mockHandler); + + const doctorCommand = program.commands.find(cmd => cmd.name() === 'doctor'); + + expect(doctorCommand).toBeDefined(); + expect(doctorCommand?.description()).toContain('toolchain'); + + const options = doctorCommand?.options || []; + const jsonOption = options.find(opt => opt.long === '--json'); + const timeoutOption = options.find(opt => opt.long === '--timeout'); + + expect(jsonOption).toBeDefined(); + expect(jsonOption?.defaultValue).toBe(false); + expect(timeoutOption).toBeDefined(); + expect(timeoutOption?.defaultValue).toBe('10000'); + }); + + it('should pass requested languages and parsed options to the handler', async () => { + const program = new Command(); + const mockHandler = vi.fn().mockResolvedValue(undefined); + + setupDoctorCommand(program, mockHandler); + + await program.parseAsync(['node', 'test', 'doctor', 'python', 'go', '--json']); + + expect(mockHandler).toHaveBeenCalledWith( + ['python', 'go'], + expect.objectContaining({ json: true, timeout: '10000' }), + expect.anything() + ); + }); + + it('should pass an empty language list when none are requested', async () => { + const program = new Command(); + const mockHandler = vi.fn().mockResolvedValue(undefined); + + setupDoctorCommand(program, mockHandler); + + await program.parseAsync(['node', 'test', 'doctor']); + + expect(mockHandler).toHaveBeenCalledWith( + [], + expect.objectContaining({ json: false, timeout: '10000' }), + expect.anything() + ); + }); + }); + describe('Integration', () => { it('should set stdio as default command', async () => { const program = new Command(); @@ -208,5 +267,19 @@ describe('CLI Setup', () => { expect(stdioHandler).toHaveBeenCalled(); expect(sseHandler).not.toHaveBeenCalled(); }); + + it('should keep stdio as the default when doctor is registered', async () => { + const program = new Command(); + const stdioHandler = vi.fn().mockResolvedValue(undefined); + const doctorHandler = vi.fn().mockResolvedValue(undefined); + + setupStdioCommand(program, stdioHandler); + setupDoctorCommand(program, doctorHandler); + + await program.parseAsync(['node', 'test']); + + expect(stdioHandler).toHaveBeenCalled(); + expect(doctorHandler).not.toHaveBeenCalled(); + }); }); }); diff --git a/tests/unit/index.test.ts b/tests/unit/index.test.ts index 1bf46b96..3aee1526 100644 --- a/tests/unit/index.test.ts +++ b/tests/unit/index.test.ts @@ -17,6 +17,9 @@ vi.mock('../../src/cli/setup.js'); vi.mock('../../src/cli/stdio-command.js'); vi.mock('../../src/cli/sse-command.js'); vi.mock('../../src/cli/version.js'); +vi.mock('../../src/cli/commands/doctor/index.js', () => ({ + handleDoctorCommand: vi.fn().mockResolvedValue(1) +})); describe('index.ts', () => { let mockLogger: any; @@ -138,6 +141,30 @@ describe('index.ts', () => { expect(runStartupJanitor).toHaveBeenCalledTimes(1); }); + it('wires the doctor command, propagates its exit code, and skips the janitor (issue #423)', async () => { + let capturedHandler: any; + vi.mocked(setup.setupDoctorCommand).mockImplementation((program, handler) => { + capturedHandler = handler; + }); + const { handleDoctorCommand } = await import('../../src/cli/commands/doctor/index.js'); + const previousExitCode = process.exitCode; + + try { + await main(); + + expect(setup.setupDoctorCommand).toHaveBeenCalledWith(mockProgram, expect.any(Function)); + + await capturedHandler(['python'], { json: true }); + + expect(handleDoctorCommand).toHaveBeenCalledWith(['python'], { json: true }); + expect(process.exitCode).toBe(1); + // Doctor is a diagnostic, not a server: no orphan reaping (issue #399 contract) + expect(runStartupJanitor).not.toHaveBeenCalled(); + } finally { + process.exitCode = previousExitCode; + } + }); + it('should pass correct handlers to setupSSECommand', async () => { let capturedHandler: any; vi.mocked(setup.setupSSECommand).mockImplementation((program, handler) => { From b160d0ebf47a68969672669d2ded7bf6741842d4 Mon Sep 17 00:00:00 2001 From: JF Date: Sat, 22 Aug 2026 19:03:32 -0400 Subject: [PATCH 3/6] test(doctor): use allowlisted generic path in presenter fixture --- tests/unit/cli/doctor/presenters.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/cli/doctor/presenters.test.ts b/tests/unit/cli/doctor/presenters.test.ts index 601939ff..d354bcb3 100644 --- a/tests/unit/cli/doctor/presenters.test.ts +++ b/tests/unit/cli/doctor/presenters.test.ts @@ -34,7 +34,7 @@ describe('presentLanguage', () => { const view = presentLanguage('go', { goPath: '/usr/local/go/bin/go', goVersion: '1.24.1', - dlvPath: '/home/u/go/bin/dlv', + dlvPath: '/home/user/go/bin/dlv', dlvVersion: '1.26.3' }); From 5932fd8e8891730d04f0691b97b810eede724310 Mon Sep 17 00:00:00 2001 From: JF Date: Sat, 22 Aug 2026 19:21:38 -0400 Subject: [PATCH 4/6] fix(doctor): partially usable adapters are warn, not broken (#423) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found running doctor in the Linux container: ruby reported broken there, but container ruby is attach-only by design — direct-connect attach runs the debug engine inside the debuggee and needs no local toolchain, and list_supported_languages reports attach as available. Gating a container job on 'doctor ruby' would have failed falsely. When the launch toolchain probe fails but attach remains available, the verdict is now warn (launch errors still listed, plus an explicit note), and a gated run passes. Verdict stays broken when neither mode works. Co-Authored-By: Claude Fable 5 --- src/cli/commands/doctor/diagnose.ts | 19 +++++++++++--- tests/unit/cli/doctor/diagnose.test.ts | 36 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/cli/commands/doctor/diagnose.ts b/src/cli/commands/doctor/diagnose.ts index 38eb0824..d174553b 100644 --- a/src/cli/commands/doctor/diagnose.ts +++ b/src/cli/commands/doctor/diagnose.ts @@ -271,9 +271,22 @@ async function diagnoseLanguage( ]; warnings = []; } else if (!validation.valid) { - verdict = 'broken'; - errors = validation.errors; - warnings = validation.warnings; + // A failed toolchain probe kills launch, but direct-connect attach runs + // the debug engine inside the debuggee and needs nothing local (container + // ruby is attach-only by design) — a partially usable adapter is a warn, + // not broken, and must not fail a gated run. + if (modes.attach.available) { + verdict = 'warn'; + errors = validation.errors; + warnings = [ + ...validation.warnings, + `Launch is unavailable, but attach (direct-connect) still works — see the errors above for what launch would need.` + ]; + } else { + verdict = 'broken'; + errors = validation.errors; + warnings = validation.warnings; + } } else { verdict = validation.warnings.length > 0 ? 'warn' : 'ok'; errors = []; diff --git a/tests/unit/cli/doctor/diagnose.test.ts b/tests/unit/cli/doctor/diagnose.test.ts index bc4601f5..fa38b5d8 100644 --- a/tests/unit/cli/doctor/diagnose.test.ts +++ b/tests/unit/cli/doctor/diagnose.test.ts @@ -125,6 +125,42 @@ describe('diagnose', () => { expect(report.languages[0].modes?.launch.available).toBe(false); }); + it('downgrades broken to warn when attach remains available (container ruby: attach-only by design)', async () => { + const deps = makeDeps([ + { + name: 'ruby', + attach: 'direct-connect', + validate: async () => ({ valid: false, errors: ['Ruby not found.'], warnings: [], details: {} }) + } + ]); + + const report = await diagnose([], deps); + + const ruby = report.languages[0]; + expect(ruby.verdict).toBe('warn'); + expect(ruby.errors[0]).toContain('Ruby not found'); + expect(ruby.warnings.some((w) => w.includes('attach'))).toBe(true); + expect(ruby.modes?.launch.available).toBe(false); + expect(ruby.modes?.attach.available).toBe(true); + // Gating on such a language must pass: its supported mode works. + await expect(diagnose(['ruby'], deps)).resolves.toMatchObject({ exitCode: 0 }); + }); + + it('keeps broken when neither launch nor attach is available', async () => { + const deps = makeDeps([ + { + name: 'go', + attach: 'none', + validate: async () => ({ valid: false, errors: ['Delve not found.'], warnings: [], details: {} }) + } + ]); + + const report = await diagnose(['go'], deps); + + expect(report.languages[0].verdict).toBe('broken'); + expect(report.exitCode).toBe(1); + }); + it('reports missing for adapters that are not installed', async () => { const deps = makeDeps([{ name: 'ruby', installed: false }]); From be105b3867e66bdb54a024f4bf407d3d61841d7e Mon Sep 17 00:00:00 2001 From: JF Date: Sat, 22 Aug 2026 19:58:50 -0400 Subject: [PATCH 5/6] =?UTF-8?q?fix(doctor):=20review=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20probe=20hygiene,=20bundle=20imports,=20honest=20ver?= =?UTF-8?q?dicts=20(#423)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review batch (14 findings addressed; #435 filed for the two architectural ones): Probe correctness: - All five new spawn probes (getDebugpyVersion, getNetcoredbgVersion, getDotnetSdkVersion, cpp captureVersionLine, java validate probe) now read output on 'close' instead of 'exit' — the stdio-drain race this repo already fixed once in ruby-utils (a3993162) — and kill their child after a 10s guard timeout so probes never strand processes. Test spawn fakes across six files now emit 'close' like real children. - java: validateJavacExecutable was a line-for-line copy of validateJavaExecutable — deleted; findJavacExecutable reuses the shared probe. python: utils-level hasDebugpy delegates to getDebugpyVersion (adapter's cached copy left; its tests replace the utils module wholesale). cpp: getCompilerInfo(command?) skips the candidate re-scan when validate() already found the compiler. Doctor behavior: - npx bundle: collectDoctorExtras' variable-specifier dynamic import can never resolve in the published CLI (adapters are bundled, not dependencies) — switched to literal-specifier imports esbuild can inline. Verified by installing the packed tarball into a bare dir: all four extras now resolve with no adapter packages on disk. - A hung extras child now sets probe.timedOut so the handler's force-exit containment covers it; extras run inside the language's remaining timeout budget and probe.durationMs spans both phases. - --timeout is parsed as strict digits ('1e4'/'10s' are usage errors, not 1ms/10ms probe budgets). - An installed adapter whose factory fails to load is broken (exit 1 when requested), not warn — with the load error surfaced. - factory.getMetadata() is fault-isolated so one malformed factory cannot collapse the whole report. - Platform check warns when MCP_CONTAINER is set to a truthy-looking but unrecognized value (=1/TRUE) instead of blessing it as host mode. - presentLanguage only renders a runtime/backend cell when something was detected — an absent toolchain now shows '—' instead of its label. - CLAUDE.md new-adapter checklist gains the doctor wiring + smoke-count steps. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 7 +- .../adapter-cpp/src/utils/compile-utils.ts | 34 ++++++-- .../adapter-cpp/tests/compile-utils.test.ts | 52 ++++++++++++ .../adapter-dotnet/src/utils/dotnet-utils.ts | 84 +++++++++++-------- .../tests/unit/dotnet-utils.test.ts | 38 +++++++++ packages/adapter-java/src/utils/java-utils.ts | 63 ++++++-------- .../adapter-python/src/utils/python-utils.ts | 70 +++++++++------- .../unit/python-utils.comprehensive.test.ts | 42 ++++++++++ .../tests/unit/python-utils.discovery.test.ts | 4 +- src/cli/commands/doctor/diagnose.ts | 51 ++++++++--- src/cli/commands/doctor/index.ts | 6 +- src/cli/commands/doctor/platform-checks.ts | 15 ++++ src/cli/commands/doctor/presenters.ts | 82 +++++++++++++----- .../java/unit/java-adapter-factory.test.ts | 12 +-- .../java/unit/java-debug-adapter.test.ts | 18 ++-- tests/adapters/java/unit/java-utils.test.ts | 33 ++++++-- tests/unit/cli/doctor/diagnose.test.ts | 71 ++++++++++++++++ tests/unit/cli/doctor/index.test.ts | 29 ++++--- tests/unit/cli/doctor/platform-checks.test.ts | 25 ++++++ tests/unit/cli/doctor/presenters.test.ts | 31 +++++-- 20 files changed, 570 insertions(+), 197 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1dafd085..321b1a97 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -313,9 +313,10 @@ To add support for a new language: 3. **Export Factory**: Export a factory class named `{Language}AdapterFactory` 4. **Register in root `package.json`**: Add `"@debugmcp/adapter-{language}": "workspace:*"` to `optionalDependencies` 5. **Add Vitest alias**: Add `{ find: '@debugmcp/adapter-{language}', replacement: path.resolve(__dirname, './packages/adapter-{language}/src/index.ts') }` to `resolve.alias` in `vitest.config.ts` -6. **Update adapter count**: Update hardcoded adapter counts in tests (`adapter-loader.test.ts`, `models.test.ts`) -7. **Add Tests**: Include unit and integration tests in the package -8. **Run `pnpm install`**: To link the new workspace package +6. **Update adapter count**: Update hardcoded adapter counts in tests (`adapter-loader.test.ts`, `models.test.ts`, `tests/e2e/doctor-smoke.test.ts`) +7. **Wire the doctor command**: Add the language's runtime/backend column mapping to `presentLanguage` in `src/cli/commands/doctor/presenters.ts` (and `collectDoctorExtras` if it has doctor-only probes) +8. **Add Tests**: Include unit and integration tests in the package +9. **Run `pnpm install`**: To link the new workspace package Example structure: ``` diff --git a/packages/adapter-cpp/src/utils/compile-utils.ts b/packages/adapter-cpp/src/utils/compile-utils.ts index 4c06857d..9d0c0896 100644 --- a/packages/adapter-cpp/src/utils/compile-utils.ts +++ b/packages/adapter-cpp/src/utils/compile-utils.ts @@ -78,17 +78,22 @@ export interface CompilerInfo { version: string | null; } +/** Upper bound for a single toolchain probe child before it is killed. */ +const PROBE_KILL_TIMEOUT_MS = 10_000; + /** - * Discover the available compiler and its version banner (the first line of - * `--version` output, e.g. "g++ (MinGW-w64 ...) 13.2.0"). Doctor-only probe - * (issue #423) — validate() keeps its cheaper presence-only findAnyCompiler. + * The available compiler and its version banner (the first line of + * `--version` output, e.g. "g++ (MinGW-w64 ...) 13.2.0"). Pass an already + * discovered `command` (e.g. validate()'s details.compiler) to skip the + * candidate scan. Doctor-only probe (issue #423) — validate() keeps its + * cheaper presence-only findAnyCompiler. */ -export async function getCompilerInfo(): Promise { - const command = await findAnyCompiler(); - if (!command) { +export async function getCompilerInfo(command?: string): Promise { + const resolved = command ?? (await findAnyCompiler()); + if (!resolved) { return null; } - return { command, version: await captureVersionLine(command) }; + return { command: resolved, version: await captureVersionLine(resolved) }; } function captureVersionLine(command: string): Promise { @@ -101,8 +106,19 @@ function captureVersionLine(command: string): Promise { let output = ''; child.stdout?.on('data', (data) => { output += data.toString(); }); child.stderr?.on('data', (data) => { output += data.toString(); }); - child.on('error', () => resolve(null)); - child.on('exit', (code) => { + + const killTimer = setTimeout(() => { + try { child.kill(); } catch { /* already gone */ } + }, PROBE_KILL_TIMEOUT_MS); + killTimer.unref?.(); + + child.on('error', () => { + clearTimeout(killTimer); + resolve(null); + }); + // 'close' (not 'exit') so stdio is fully drained before reading output + child.on('close', (code) => { + clearTimeout(killTimer); const firstLine = output.split(/\r?\n/).map((line) => line.trim()).find((line) => line.length > 0); resolve(code === 0 && firstLine ? firstLine : null); }); diff --git a/packages/adapter-cpp/tests/compile-utils.test.ts b/packages/adapter-cpp/tests/compile-utils.test.ts index 9c7e35fd..cfc9925b 100644 --- a/packages/adapter-cpp/tests/compile-utils.test.ts +++ b/packages/adapter-cpp/tests/compile-utils.test.ts @@ -40,6 +40,8 @@ function fakeProcess(exitCode: number, stderr = '', stdout = ''): EventEmitter & proc.stdout.emit('data', Buffer.from(stdout)); } proc.emit('exit', exitCode); + // Real children emit 'close' after 'exit' once stdio drains + proc.emit('close', exitCode); }); return proc; } @@ -170,6 +172,56 @@ describe('compile-utils', () => { await expect(getCompilerInfo()).resolves.toEqual({ command: 'g++', version: null }); }); + + it('skips candidate discovery when the command is already known', async () => { + spawnMock.mockImplementationOnce(() => + fakeProcess(0, '', 'clang++ version 17.0.1\n') + ); + + await expect(getCompilerInfo('clang++')).resolves.toEqual({ + command: 'clang++', + version: 'clang++ version 17.0.1' + }); + expect(spawnMock).toHaveBeenCalledTimes(1); + expect(spawnMock.mock.calls[0][0]).toBe('clang++'); + }); + + it("still sees stdout that arrives between 'exit' and 'close' (stdio drain race)", async () => { + spawnMock.mockImplementationOnce(() => { + const proc = new EventEmitter() as EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; kill: () => void }; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.kill = vi.fn(); + setImmediate(() => { + proc.emit('exit', 0); + proc.stdout.emit('data', Buffer.from('g++ 13.2.0\n')); + proc.emit('close', 0); + }); + return proc; + }); + + await expect(getCompilerInfo('g++')).resolves.toEqual({ command: 'g++', version: 'g++ 13.2.0' }); + }); + + it('kills a hung version capture after the guard timeout', async () => { + vi.useFakeTimers(); + const kill = vi.fn(); + let proc!: EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; kill: typeof kill }; + spawnMock.mockImplementationOnce(() => { + proc = new EventEmitter() as typeof proc; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.kill = kill; + return proc; + }); + + const pending = getCompilerInfo('g++'); + await vi.advanceTimersByTimeAsync(10_100); + expect(kill).toHaveBeenCalled(); + proc.emit('close', null); + await expect(pending).resolves.toEqual({ command: 'g++', version: null }); + vi.useRealTimers(); + }); }); describe('getDefaultOutputPath', () => { diff --git a/packages/adapter-dotnet/src/utils/dotnet-utils.ts b/packages/adapter-dotnet/src/utils/dotnet-utils.ts index 524bd7f1..7a767ebc 100644 --- a/packages/adapter-dotnet/src/utils/dotnet-utils.ts +++ b/packages/adapter-dotnet/src/utils/dotnet-utils.ts @@ -200,35 +200,36 @@ export async function findDotnetBackend( return { backend: 'netcoredbg', path: netcoredbgPath }; } +/** Upper bound for a single toolchain probe child before it is killed. */ +const PROBE_KILL_TIMEOUT_MS = 10_000; + /** - * Report the version of a netcoredbg executable, or null when it cannot be - * spawned or exits non-zero. Extracts the version token from output like - * "NET Core debugger 3.1.2-1054"; falls back to the first non-empty output - * line for builds that print a different banner. Doctor-only probe (issue - * #423) — not called from validate(), so registration cost is unchanged. + * Run a --version-style probe, resolving with drained stdout on 'close' + * (never 'exit' — the final chunk can arrive after it), killing the child if + * it hangs past the guard timeout. Null on spawn failure or non-zero exit. */ -export async function getNetcoredbgVersion(netcoredbgPath: string): Promise { +function runVersionProbe(command: string, args: string[]): Promise { return new Promise((resolve) => { try { - const child = spawn(netcoredbgPath, ['--version'], { + const child = spawn(command, args, { stdio: ['ignore', 'pipe', 'pipe'], windowsHide: true }); let output = ''; child.stdout?.on('data', (data) => { output += data.toString(); }); - child.on('error', () => resolve(null)); - child.on('exit', (code) => { - if (code !== 0) { - resolve(null); - return; - } - const match = output.match(/(\d+\.\d+\.\d+(?:-[\w.]+)?)/); - if (match) { - resolve(match[1]); - return; - } - const firstLine = output.split(/\r?\n/).map((line) => line.trim()).find((line) => line.length > 0); - resolve(firstLine ?? null); + + const killTimer = setTimeout(() => { + try { child.kill(); } catch { /* already gone */ } + }, PROBE_KILL_TIMEOUT_MS); + killTimer.unref?.(); + + child.on('error', () => { + clearTimeout(killTimer); + resolve(null); + }); + child.on('close', (code) => { + clearTimeout(killTimer); + resolve(code === 0 ? output : null); }); } catch { resolve(null); @@ -236,29 +237,38 @@ export async function getNetcoredbgVersion(netcoredbgPath: string): Promise { + const output = await runVersionProbe(netcoredbgPath, ['--version']); + if (output === null) { + return null; + } + const match = output.match(/(\d+\.\d+\.\d+(?:-[\w.]+)?)/); + if (match) { + return match[1]; + } + const firstLine = output.split(/\r?\n/).map((line) => line.trim()).find((line) => line.length > 0); + return firstLine ?? null; +} + /** * Report the active .NET SDK version (`dotnet --version`), or null when the * dotnet CLI is missing or reports an error (e.g. no SDK behind a bare * runtime install). Doctor-only probe (issue #423). */ export async function getDotnetSdkVersion(): Promise { - return new Promise((resolve) => { - try { - const child = spawn('dotnet', ['--version'], { - stdio: ['ignore', 'pipe', 'pipe'], - windowsHide: true - }); - let output = ''; - child.stdout?.on('data', (data) => { output += data.toString(); }); - child.on('error', () => resolve(null)); - child.on('exit', (code) => { - const version = output.trim().split(/\r?\n/)[0]?.trim() ?? ''; - resolve(code === 0 && version.length > 0 ? version : null); - }); - } catch { - resolve(null); - } - }); + const output = await runVersionProbe('dotnet', ['--version']); + if (output === null) { + return null; + } + const version = output.trim().split(/\r?\n/)[0]?.trim() ?? ''; + return version.length > 0 ? version : null; } /** diff --git a/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts b/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts index 109ee719..f951a9ee 100644 --- a/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts +++ b/packages/adapter-dotnet/tests/unit/dotnet-utils.test.ts @@ -78,6 +78,15 @@ const spawnMock = spawn as unknown as vi.Mock; const spawnSyncMock = spawnSync as unknown as vi.Mock; const whichMock = which as unknown as vi.Mock; +const createSpawnShell = (): ChildProcessMock => { + const proc = new EventEmitter() as ChildProcessMock; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.stdin = new EventEmitter(); + proc.kill = vi.fn(); + return proc; +}; + const createSpawn = (options: { exitCode: number; stdout?: string; stderr?: string; error?: Error }) => { const proc = new EventEmitter() as ChildProcessMock; proc.stdout = new EventEmitter(); @@ -97,6 +106,8 @@ const createSpawn = (options: { exitCode: number; stdout?: string; stderr?: stri proc.stderr.emit('data', Buffer.from(options.stderr)); } proc.emit('exit', options.exitCode); + // Real children emit 'close' after 'exit' once stdio drains + proc.emit('close', options.exitCode); }); return proc; @@ -139,6 +150,33 @@ describe('getNetcoredbgVersion (issue #423)', () => { await expect(getNetcoredbgVersion('/missing/netcoredbg')).resolves.toBeNull(); }); + + it("still sees stdout that arrives between 'exit' and 'close' (stdio drain race)", async () => { + spawnMock.mockImplementation(() => { + const proc = createSpawnShell(); + setImmediate(() => { + proc.emit('exit', 0); + proc.stdout.emit('data', Buffer.from('NET Core debugger 3.1.2-1054\n')); + proc.emit('close', 0); + }); + return proc; + }); + + await expect(getNetcoredbgVersion('/opt/netcoredbg/netcoredbg')).resolves.toBe('3.1.2-1054'); + }); + + it('kills a hung probe child after the guard timeout and resolves null', async () => { + vi.useFakeTimers(); + const proc = createSpawnShell(); + spawnMock.mockImplementation(() => proc); + + const pending = getNetcoredbgVersion('/opt/netcoredbg/netcoredbg'); + await vi.advanceTimersByTimeAsync(10_100); + expect(proc.kill).toHaveBeenCalled(); + proc.emit('close', null); + await expect(pending).resolves.toBeNull(); + vi.useRealTimers(); + }); }); describe('getDotnetSdkVersion (issue #423)', () => { diff --git a/packages/adapter-java/src/utils/java-utils.ts b/packages/adapter-java/src/utils/java-utils.ts index 18014d6b..6cd0288c 100644 --- a/packages/adapter-java/src/utils/java-utils.ts +++ b/packages/adapter-java/src/utils/java-utils.ts @@ -39,8 +39,13 @@ export async function findJavaExecutable(preferredPath?: string): Promise -version`. Reads on 'close' (not 'exit') so late stdio chunks still + * count as output, and kills the child if it hangs past the guard timeout. */ export async function validateJavaExecutable(javaPath: string): Promise { return new Promise((resolve) => { @@ -54,14 +59,22 @@ export async function validateJavaExecutable(javaPath: string): Promise let hasOutput = false; child.stderr?.on('data', () => { hasOutput = true; }); child.stdout?.on('data', () => { hasOutput = true; }); + + const killTimer = setTimeout(() => { + try { child.kill(); } catch { /* already gone */ } + }, PROBE_KILL_TIMEOUT_MS); + killTimer.unref?.(); + child.on('error', () => { if (settled) return; settled = true; + clearTimeout(killTimer); resolve(false); }); - child.on('exit', (code) => { + child.on('close', (code) => { if (settled) return; settled = true; + clearTimeout(killTimer); resolve(code === 0 && hasOutput); }); } catch { @@ -94,46 +107,14 @@ export async function findJavacExecutable(javaPath?: string): Promise { - return new Promise((resolve) => { - let settled = false; - try { - const child = spawn(javacPath, ['-version'], { - stdio: ['ignore', 'pipe', 'pipe'], - windowsHide: true, - }); - - let hasOutput = false; - child.stdout?.on('data', () => { hasOutput = true; }); - child.stderr?.on('data', () => { hasOutput = true; }); - child.on('error', () => { - if (settled) return; - settled = true; - resolve(false); - }); - child.on('exit', (code) => { - if (settled) return; - settled = true; - resolve(code === 0 && hasOutput); - }); - } catch { - if (settled) return; - settled = true; - resolve(false); - } - }); -} - /** * Get the Java version string. */ @@ -157,14 +138,22 @@ export async function getJavaVersion(javaPath?: string): Promise output += data.toString(); }); + const killTimer = setTimeout(() => { + try { child.kill(); } catch { /* already gone */ } + }, PROBE_KILL_TIMEOUT_MS); + killTimer.unref?.(); + child.on('error', () => { if (settled) return; settled = true; + clearTimeout(killTimer); resolve(null); }); - child.on('exit', (code) => { + // 'close' (not 'exit') so stdio is fully drained before parsing + child.on('close', (code) => { if (settled) return; settled = true; + clearTimeout(killTimer); if (code !== 0) { resolve(null); return; diff --git a/packages/adapter-python/src/utils/python-utils.ts b/packages/adapter-python/src/utils/python-utils.ts index 317b76b9..65c3d0c3 100644 --- a/packages/adapter-python/src/utils/python-utils.ts +++ b/packages/adapter-python/src/utils/python-utils.ts @@ -277,27 +277,17 @@ async function isValidPythonExecutable(pythonCmd: string, logger: Logger = noopL } /** - * Check if a Python executable has debugpy installed + * Check if a Python executable has debugpy installed. Delegates to the shared + * getDebugpyVersion probe so the spawn command cannot drift between callers. + * (PythonDebugAdapter.checkDebugpyInstalled keeps its own cached copy for now + * — its tests replace this module wholesale.) */ async function hasDebugpy(pythonPath: string, logger: Logger = noopLogger): Promise { - return new Promise((resolve) => { - const child = spawn(pythonPath, ['-c', 'import debugpy; print(debugpy.__version__)'], { - stdio: ['ignore', 'pipe', 'pipe'], - windowsHide: true - }); - - let output = ''; - child.stdout?.on('data', (data) => { output += data.toString(); }); - - child.on('error', () => resolve(false)); - child.on('exit', (code) => { - const hasIt = code === 0 && output.trim().length > 0; - if (hasIt) { - logger.debug?.(`[Python Detection] debugpy version: ${sanitizeStderrTail(output)}`); - } - resolve(hasIt); - }); - }); + const version = await getDebugpyVersion(pythonPath); + if (version !== null) { + logger.debug?.(`[Python Detection] debugpy version: ${sanitizeStderrTail(version)}`); + } + return version !== null; } /** @@ -488,26 +478,46 @@ export async function findPythonExecutable( /** * Get Python version for a given executable */ +/** Upper bound for a single toolchain probe child before it is killed. */ +const PROBE_KILL_TIMEOUT_MS = 10_000; + /** * Report the debugpy version importable by the given interpreter, or null when * debugpy is missing (or the interpreter cannot be spawned). Null is not fatal: * debugpy may still be available in the user's virtualenv (issue #16). + * + * Reads output on 'close' (not 'exit') so stdio is fully drained, and kills + * the child if it hangs past the guard timeout — probes must never strand + * processes (Development Guidelines #8). */ export async function getDebugpyVersion(pythonPath: string): Promise { return new Promise((resolve) => { - const child = spawn(pythonPath, ['-c', 'import debugpy; print(debugpy.__version__)'], { - stdio: ['ignore', 'pipe', 'pipe'], - windowsHide: true - }); + try { + const child = spawn(pythonPath, ['-c', 'import debugpy; print(debugpy.__version__)'], { + stdio: ['ignore', 'pipe', 'pipe'], + windowsHide: true + }); - let output = ''; - child.stdout?.on('data', (data) => { output += data.toString(); }); + let output = ''; + child.stdout?.on('data', (data) => { output += data.toString(); }); - child.on('error', () => resolve(null)); - child.on('exit', (code) => { - const version = output.trim(); - resolve(code === 0 && version.length > 0 ? version : null); - }); + const killTimer = setTimeout(() => { + try { child.kill(); } catch { /* already gone */ } + }, PROBE_KILL_TIMEOUT_MS); + killTimer.unref?.(); + + child.on('error', () => { + clearTimeout(killTimer); + resolve(null); + }); + child.on('close', (code) => { + clearTimeout(killTimer); + const version = output.trim(); + resolve(code === 0 && version.length > 0 ? version : null); + }); + } catch { + resolve(null); + } }); } diff --git a/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts b/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts index c4b0e4ff..d97520cc 100644 --- a/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts +++ b/packages/adapter-python/tests/unit/python-utils.comprehensive.test.ts @@ -54,6 +54,8 @@ const createSpawn = (options: { exitCode: number; stdout?: string; stderr?: stri proc.stderr.emit('data', Buffer.from(options.stderr)); } proc.emit('exit', options.exitCode); + // Real children emit 'close' after 'exit' once stdio drains + proc.emit('close', options.exitCode); }); return proc; @@ -602,6 +604,46 @@ describe('getDebugpyVersion (issue #423)', () => { await expect(getDebugpyVersion('/usr/bin/python3')).resolves.toBeNull(); }); + + it("still sees stdout that arrives between 'exit' and 'close' (stdio drain race)", async () => { + // Node documents that stdio may still be open when 'exit' fires: the + // final chunk can land after it. Reading on 'close' (the ruby-utils fix) + // is the only safe pattern. + spawnMock.mockImplementation(() => { + const proc = new EventEmitter() as EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; kill: () => void }; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.kill = vi.fn(); + setImmediate(() => { + proc.emit('exit', 0); + proc.stdout.emit('data', Buffer.from('1.8.14\n')); + proc.emit('close', 0); + }); + return proc; + }); + + await expect(getDebugpyVersion('/usr/bin/python3')).resolves.toBe('1.8.14'); + }); + + it('kills a hung probe child after the guard timeout and resolves null', async () => { + vi.useFakeTimers(); + const kill = vi.fn(); + let proc!: EventEmitter & { stdout: EventEmitter; stderr: EventEmitter; kill: typeof kill }; + spawnMock.mockImplementation(() => { + proc = new EventEmitter() as typeof proc; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + proc.kill = kill; + return proc; + }); + + const pending = getDebugpyVersion('/usr/bin/python3'); + await vi.advanceTimersByTimeAsync(10_100); + expect(kill).toHaveBeenCalled(); + proc.emit('close', null); + await expect(pending).resolves.toBeNull(); + vi.useRealTimers(); + }); }); describe('WhichCommandFinder class behavior', () => { diff --git a/packages/adapter-python/tests/unit/python-utils.discovery.test.ts b/packages/adapter-python/tests/unit/python-utils.discovery.test.ts index 68409a0d..ebdaa3a7 100644 --- a/packages/adapter-python/tests/unit/python-utils.discovery.test.ts +++ b/packages/adapter-python/tests/unit/python-utils.discovery.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import path from 'node:path'; import fs from 'node:fs'; import { EventEmitter } from 'node:events'; @@ -38,7 +38,7 @@ const createSpawn = (options: { exitCode: number; stdout?: string; stderr?: stri if (options.stderr) { proc.stderr.emit('data', Buffer.from(options.stderr)); } - proc.emit('exit', options.exitCode); + proc.emit('exit', options.exitCode); proc.emit('close', options.exitCode); }); return proc; diff --git a/src/cli/commands/doctor/diagnose.ts b/src/cli/commands/doctor/diagnose.ts index d174553b..827c80ee 100644 --- a/src/cli/commands/doctor/diagnose.ts +++ b/src/cli/commands/doctor/diagnose.ts @@ -195,11 +195,17 @@ async function diagnoseLanguage( }; } - const factory = await deps.registry.getFactory(entry.name).catch(() => undefined); + let factoryLoadError: unknown; + const factory = await deps.registry.getFactory(entry.name).catch((error: unknown) => { + factoryLoadError = error; + return undefined; + }); if (!factory || typeof factory.validate !== 'function') { - // Mirrors the server: no factory means no probe, and availability is - // assumed. Doctor surfaces that it could not actually look. + // An installed adapter whose factory cannot even be loaded cannot start + // any session — that is broken, and a gated run must fail. (The server + // fails open here; the modes below reflect that so the divergence stays + // visible rather than silent.) const modes = await computeModeAvailability({ language: entry.name, packageName: entry.packageName, @@ -208,13 +214,19 @@ async function diagnoseLanguage( attach: entry.attach ?? 'none', logger: deps.logger }); + const loadDetail = + factoryLoadError instanceof Error ? `: ${factoryLoadError.message}` : ''; return { ...base, - verdict: 'warn', - errors: [], - warnings: ['Adapter factory could not be loaded, so the toolchain was not probed'], + verdict: 'broken', + errors: [ + `Adapter factory could not be loaded${loadDetail} — the installed ${entry.packageName} ` + + `may be corrupt or version-skewed; try reinstalling it. ` + + `(The server assumes availability when it cannot probe.)` + ], + warnings: [], modes, - probe: { durationMs: 0, timedOut: false, failed: false } + probe: { durationMs: 0, timedOut: false, failed: true } }; } @@ -228,17 +240,24 @@ async function diagnoseLanguage( probeError = error; timedOut = error instanceof ProbeTimeoutError; } - const durationMs = Date.now() - started; const failed = probeError !== undefined && !timedOut; // Feed computeModeAvailability the same outcome the server would see: the - // memoized result, or a throwing probe so its fail-open path runs. + // memoized result, or a throwing probe so its fail-open path runs. A + // throwing getMetadata (malformed third-party factory) must not take the + // other languages down with it. + let metadataAttach: AttachMechanism | undefined; + try { + metadataAttach = factory.getMetadata().modes?.attach; + } catch { + metadataAttach = undefined; + } const modes = await computeModeAvailability({ language: entry.name, packageName: entry.packageName, installed: true, disabled: false, - attach: factory.getMetadata().modes?.attach ?? entry.attach ?? 'none', + attach: metadataAttach ?? entry.attach ?? 'none', validate: validation ? async () => validation : async () => { @@ -249,15 +268,23 @@ async function diagnoseLanguage( let details = validation?.details ? { ...validation.details } : undefined; if (validation) { + // Extras share the language's timeout budget: whatever validate() left + // over. A hung extras child is flagged via probe.timedOut so the handler's + // force-exit containment covers it too. + const remainingMs = Math.max(0, deps.timeoutMs - (Date.now() - started)); try { - const extras = await withTimeout(collectExtras(entry.name, details ?? {}), deps.timeoutMs); + const extras = await withTimeout(collectExtras(entry.name, details ?? {}), remainingMs); if (extras && Object.keys(extras).length > 0) { details = { ...(details ?? {}), ...extras }; } - } catch { + } catch (error) { // Extras are best-effort; the verdict stands on validate() alone. + if (error instanceof ProbeTimeoutError) { + timedOut = true; + } } } + const durationMs = Date.now() - started; let verdict: DoctorVerdict; let errors: string[]; diff --git a/src/cli/commands/doctor/index.ts b/src/cli/commands/doctor/index.ts index 50f613f8..c235f985 100644 --- a/src/cli/commands/doctor/index.ts +++ b/src/cli/commands/doctor/index.ts @@ -61,9 +61,11 @@ export async function handleDoctorCommand( const exit = overrides.exit ?? ((code: number) => process.exit(code)); const timeoutRaw = options.timeout ?? '10000'; - const timeoutMs = Number.parseInt(timeoutRaw, 10); + // Strict digits only: parseInt would silently truncate '1e4' to 1 and + // '10s' to 10, turning a unit typo into millisecond probe budgets. + const timeoutMs = /^\d+$/.test(timeoutRaw.trim()) ? Number(timeoutRaw.trim()) : NaN; if (!Number.isFinite(timeoutMs) || timeoutMs <= 0) { - writeError(`Invalid --timeout value: '${timeoutRaw}' (expected a positive number of milliseconds)`); + writeError(`Invalid --timeout value: '${timeoutRaw}' (expected a positive whole number of milliseconds)`); return 2; } diff --git a/src/cli/commands/doctor/platform-checks.ts b/src/cli/commands/doctor/platform-checks.ts index 0eaf9fdc..385af37a 100644 --- a/src/cli/commands/doctor/platform-checks.ts +++ b/src/cli/commands/doctor/platform-checks.ts @@ -85,6 +85,21 @@ export async function checkContainerWorkspace( const mountBase = { id: 'workspace-mount' as const, label: 'workspace mount' }; if (!isContainerMode(environment)) { + // Only the exact string 'true' enables container mode — a truthy-looking + // near-miss (MCP_CONTAINER=1/TRUE/yes) is precisely the misconfiguration + // a doctor run should call out rather than bless as "host mode". + const rawValue = environment.get('MCP_CONTAINER'); + if (rawValue !== undefined && rawValue !== '') { + return [ + { + ...containerBase, + status: 'warn', + detail: `MCP_CONTAINER='${rawValue}' is set but does not enable container mode`, + fixHint: "Container mode requires exactly MCP_CONTAINER=true" + }, + { ...mountBase, status: 'skipped', detail: 'host mode (container mode not enabled)' } + ]; + } return [ { ...containerBase, status: 'ok', detail: 'not running in container mode' }, { ...mountBase, status: 'skipped', detail: 'host mode' } diff --git a/src/cli/commands/doctor/presenters.ts b/src/cli/commands/doctor/presenters.ts index 5a944107..1ccd1d25 100644 --- a/src/cli/commands/doctor/presenters.ts +++ b/src/cli/commands/doctor/presenters.ts @@ -33,68 +33,86 @@ const str = (details: Details | undefined, key: string): string | undefined => { return typeof value === 'string' && value.length > 0 ? value : undefined; }; +/** + * A cell is only rendered when something was actually detected — a bare label + * would make an absent toolchain read as present. `(built-in)` style labels + * stand alone by design (mock). + */ +function component(info: T): T | undefined { + const { label, ...rest } = info as DoctorBackendInfo; + if (label.startsWith('(')) { + return info; + } + return Object.values(rest).some((value) => value !== undefined) ? info : undefined; +} + export function presentLanguage(language: string, details: Details | undefined): LanguageView { + if (details === undefined) { + // The probe failed or timed out before producing anything — an empty row + // is the honest rendering. + return {}; + } switch (language) { case 'python': return { - runtime: { label: 'Python', path: str(details, 'pythonPath'), version: str(details, 'pythonVersion') }, - backend: { label: 'debugpy', version: str(details, 'debugpyVersion') } + runtime: component({ label: 'Python', path: str(details, 'pythonPath'), version: str(details, 'pythonVersion') }), + backend: component({ label: 'debugpy', version: str(details, 'debugpyVersion') }) }; case 'javascript': return { - runtime: { label: 'Node.js', version: str(details, 'nodeVersion') }, + runtime: component({ label: 'Node.js', version: str(details, 'nodeVersion') }), backend: { label: 'js-debug', source: 'vendored' } }; case 'ruby': return { - runtime: { label: 'Ruby', path: str(details, 'rubyPath'), version: str(details, 'rubyVersion') }, - backend: { label: 'rdbg', path: str(details, 'rdbgPath'), version: str(details, 'rdbgVersion') } + runtime: component({ label: 'Ruby', path: str(details, 'rubyPath'), version: str(details, 'rubyVersion') }), + backend: component({ label: 'rdbg', path: str(details, 'rdbgPath'), version: str(details, 'rdbgVersion') }) }; case 'go': return { - runtime: { label: 'Go', path: str(details, 'goPath'), version: str(details, 'goVersion') }, - backend: { label: 'Delve', path: str(details, 'dlvPath'), version: str(details, 'dlvVersion') } + runtime: component({ label: 'Go', path: str(details, 'goPath'), version: str(details, 'goVersion') }), + backend: component({ label: 'Delve', path: str(details, 'dlvPath'), version: str(details, 'dlvVersion') }) }; case 'java': return { - runtime: { label: 'Java', path: str(details, 'javaPath'), version: str(details, 'javaVersion') }, - backend: { label: 'JDI bridge', path: str(details, 'jdiBridgeDir') } + runtime: component({ label: 'Java', path: str(details, 'javaPath'), version: str(details, 'javaVersion') }), + backend: component({ label: 'JDI bridge', path: str(details, 'jdiBridgeDir') }) }; case 'dotnet': return { - runtime: { label: '.NET SDK', version: str(details, 'dotnetSdkVersion') }, - backend: { + runtime: component({ label: '.NET SDK', version: str(details, 'dotnetSdkVersion') }), + backend: component({ label: 'netcoredbg', path: str(details, 'debuggerPath'), version: str(details, 'netcoredbgVersion') - } + }) }; case 'rust': return { - runtime: { label: 'Rust', version: str(details, 'cargoVersion') }, - backend: { + runtime: component({ label: 'Rust', version: str(details, 'cargoVersion') }), + backend: component({ label: 'CodeLLDB', path: str(details, 'codelldbPath'), version: str(details, 'codelldbVersion'), source: str(details, 'codelldbSource') - } + }) }; case 'cpp': { // The --version banner already names the command; only show the bare // command when no banner was captured. const compilerVersion = str(details, 'compilerVersion'); return { - runtime: { + runtime: component({ label: 'C/C++ compiler', path: compilerVersion ? undefined : str(details, 'compiler'), version: compilerVersion - }, - backend: { + }), + backend: component({ label: 'CodeLLDB', path: str(details, 'codelldbPath'), version: str(details, 'codelldbVersion'), source: str(details, 'codelldbSource') - } + }) }; } case 'mock': @@ -109,7 +127,25 @@ export function presentLanguage(language: string, details: Details | undefined): type ImportModule = (id: string) => Promise; -const defaultImportModule: ImportModule = (id) => import(/* @vite-ignore */ id); +/** + * Literal specifiers only: a variable `import(id)` cannot be inlined by the + * npx bundle's esbuild pass, so it would throw ERR_MODULE_NOT_FOUND in every + * npx/global install (the CLI package ships no adapter dependencies — they + * are bundled). With literals, esbuild bundles each target and the same code + * works in repo checkouts, Docker, and npx. + */ +const defaultImportModule: ImportModule = (id) => { + switch (id) { + case '@debugmcp/adapter-dotnet': + return import('@debugmcp/adapter-dotnet'); + case '@debugmcp/adapter-java': + return import('@debugmcp/adapter-java'); + case '@debugmcp/adapter-cpp': + return import('@debugmcp/adapter-cpp'); + default: + return Promise.reject(new Error(`No doctor extras module registered for '${id}'`)); + } +}; /** * Doctor-only probes, run per installed language after validate(). Each is @@ -153,10 +189,12 @@ export async function collectDoctorExtras( } case 'cpp': { const mod = (await importModule('@debugmcp/adapter-cpp')) as Partial<{ - getCompilerInfo(): Promise<{ command: string; version: string | null } | null>; + getCompilerInfo(command?: string): Promise<{ command: string; version: string | null } | null>; }>; if (typeof mod.getCompilerInfo === 'function') { - const info = await mod.getCompilerInfo(); + // validate() already discovered the command — reuse it instead of + // re-probing the whole candidate list. + const info = await mod.getCompilerInfo(str(details, 'compiler')); if (info?.version) return { compilerVersion: info.version }; } return {}; diff --git a/tests/adapters/java/unit/java-adapter-factory.test.ts b/tests/adapters/java/unit/java-adapter-factory.test.ts index b2cebdd8..d85845e1 100644 --- a/tests/adapters/java/unit/java-adapter-factory.test.ts +++ b/tests/adapters/java/unit/java-adapter-factory.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { EventEmitter } from 'events'; import { spawn } from 'child_process'; import type { AdapterDependencies } from '@debugmcp/shared'; @@ -97,7 +97,7 @@ describe('JavaAdapterFactory', () => { process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1" 2021-10-19\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; @@ -136,7 +136,7 @@ describe('JavaAdapterFactory', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -155,7 +155,7 @@ describe('JavaAdapterFactory', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -175,7 +175,7 @@ describe('JavaAdapterFactory', () => { process.nextTick(() => { // Simulate Java 17 which is valid but below recommended 21 proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -194,7 +194,7 @@ describe('JavaAdapterFactory', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "21.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); diff --git a/tests/adapters/java/unit/java-debug-adapter.test.ts b/tests/adapters/java/unit/java-debug-adapter.test.ts index c9e016a4..25d385f0 100644 --- a/tests/adapters/java/unit/java-debug-adapter.test.ts +++ b/tests/adapters/java/unit/java-debug-adapter.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { EventEmitter } from 'events'; import { spawn } from 'child_process'; import path from 'path'; @@ -89,7 +89,7 @@ describe('JavaDebugAdapter', () => { process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1" 2021-10-19\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; @@ -107,7 +107,7 @@ describe('JavaDebugAdapter', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -143,7 +143,7 @@ describe('JavaDebugAdapter', () => { process.nextTick(() => { // Simulate Java 8 (1.8.0) version string proc.stderr.emit('data', Buffer.from('openjdk version "1.8.0_292"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -165,7 +165,7 @@ describe('JavaDebugAdapter', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -356,13 +356,13 @@ describe('JavaDebugAdapter', () => { try { const result = fn(); - // JDI bridge is compiled — verify we got a valid command back + // JDI bridge is compiled — verify we got a valid command back expect(result.command).toBeTruthy(); expect(result.args).toBeDefined(); // Should launch java with JdiDapServer expect(result.args).toContain('JdiDapServer'); } catch (error) { - // JDI bridge not compiled — verify the error message + // JDI bridge not compiled — verify the error message expect((error as Error).message).toMatch(/JDI bridge not compiled/); } }); @@ -397,7 +397,7 @@ describe('JavaDebugAdapter', () => { expect(idx).toBeGreaterThanOrEqual(0); expect(result.args[idx + 1]).toBe('424242'); } catch (error) { - // JDI bridge not compiled in this environment — covered by other tests + // JDI bridge not compiled in this environment — covered by other tests expect((error as Error).message).toMatch(/JDI bridge not compiled/); } }); @@ -684,7 +684,7 @@ describe('JavaDebugAdapter', () => { expect(config.stopOnEntry).toBeUndefined(); expect(config.cwd).toBeUndefined(); expect(config.env).toBeUndefined(); - // No mandatory timeout — JDI bridge doesn't require it + // No mandatory timeout — JDI bridge doesn't require it expect(config.timeout).toBeUndefined(); }); }); diff --git a/tests/adapters/java/unit/java-utils.test.ts b/tests/adapters/java/unit/java-utils.test.ts index f0fc883b..18585778 100644 --- a/tests/adapters/java/unit/java-utils.test.ts +++ b/tests/adapters/java/unit/java-utils.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { spawn } from 'child_process'; import { EventEmitter } from 'events'; import path from 'path'; @@ -37,7 +37,7 @@ describe('java-utils', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -69,7 +69,7 @@ describe('java-utils', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -89,7 +89,7 @@ describe('java-utils', () => { process.nextTick(() => { if (cmd === 'java') { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); } else { proc.emit('error', new Error('ENOENT')); } @@ -127,7 +127,7 @@ describe('java-utils', () => { process.nextTick(() => { if (validPaths.includes(cmd)) { proc.stdout.emit('data', Buffer.from('javac 21.0.6\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); } else { proc.emit('error', new Error('ENOENT')); } @@ -176,7 +176,7 @@ describe('java-utils', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('openjdk version "17.0.1" 2021-10-19\nOpenJDK Runtime Environment\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -192,7 +192,7 @@ describe('java-utils', () => { proc.stderr = new EventEmitter(); process.nextTick(() => { proc.stderr.emit('data', Buffer.from('java version "1.8.0_301"\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -219,13 +219,30 @@ describe('java-utils', () => { const proc = new EventEmitter() as any; proc.stdout = new EventEmitter(); proc.stderr = new EventEmitter(); - process.nextTick(() => proc.emit('exit', 1)); + process.nextTick(() => { proc.emit('exit', 1); proc.emit('close', 1); }); return proc; }); const version = await getJavaVersion('java'); expect(version).toBeNull(); }); + + it("still sees stderr that arrives between 'exit' and 'close' (stdio drain race)", async () => { + mockSpawn.mockImplementation(() => { + const proc = new EventEmitter() as any; + proc.stdout = new EventEmitter(); + proc.stderr = new EventEmitter(); + process.nextTick(() => { + proc.emit('exit', 0); + proc.stderr.emit('data', Buffer.from('openjdk version "21.0.6"\n')); + proc.emit('close', 0); + }); + return proc; + }); + + const version = await getJavaVersion('java'); + expect(version).toBe('21.0.6'); + }); }); describe('getJavaSearchPaths', () => { diff --git a/tests/unit/cli/doctor/diagnose.test.ts b/tests/unit/cli/doctor/diagnose.test.ts index fa38b5d8..39a069d4 100644 --- a/tests/unit/cli/doctor/diagnose.test.ts +++ b/tests/unit/cli/doctor/diagnose.test.ts @@ -219,6 +219,77 @@ describe('diagnose', () => { expect(dotnet.modes?.launch.available).toBe(true); // fail-open parity }); + it('reports broken (not warn) for an installed adapter whose factory cannot be loaded, failing a gated run', async () => { + // installed: true but no validate => the fake registry returns undefined + // from getFactory, modelling a corrupt/version-skewed adapter package. + const deps = makeDeps([{ name: 'python', installed: true }]); + + const report = await diagnose(['python'], deps); + + const python = report.languages[0]; + expect(python.verdict).toBe('broken'); + expect(python.probe.failed).toBe(true); + expect(python.errors[0]).toContain('factory'); + expect(report.exitCode).toBe(1); + // Fail-open parity: the server would still assume availability here. + expect(python.modes?.launch.available).toBe(true); + }); + + it('sets probe.timedOut when the extras collector hangs, so the handler can force-exit', async () => { + vi.useFakeTimers(); + const deps = makeDeps( + [{ name: 'dotnet', validate: okValidate({ debuggerPath: '/x' }) }], + { + timeoutMs: 1000, + collectExtras: () => new Promise(() => undefined) + } + ); + + const reportPromise = diagnose([], deps); + await vi.advanceTimersByTimeAsync(2200); + const report = await reportPromise; + + const dotnet = report.languages[0]; + expect(dotnet.verdict).toBe('ok'); // extras are best-effort; the verdict stands on validate() + expect(dotnet.probe.timedOut).toBe(true); + }); + + it('counts the extras phase inside probe.durationMs', async () => { + vi.useFakeTimers(); + const deps = makeDeps( + [{ name: 'dotnet', validate: okValidate({ debuggerPath: '/x' }) }], + { + timeoutMs: 5000, + collectExtras: () => + new Promise((resolve) => setTimeout(() => resolve({ dotnetSdkVersion: '8.0.301' }), 300)) + } + ); + + const reportPromise = diagnose([], deps); + await vi.advanceTimersByTimeAsync(400); + const report = await reportPromise; + + expect(report.languages[0].probe.durationMs).toBeGreaterThanOrEqual(300); + }); + + it('survives a factory whose getMetadata throws, falling back to the registry attach mechanism', async () => { + const deps = makeDeps([{ name: 'ruby', attach: 'direct-connect', validate: okValidate() }]); + const registry = deps.registry as unknown as { getFactory: ReturnType }; + const originalGetFactory = registry.getFactory; + registry.getFactory = vi.fn(async (language: string) => { + const factory = await originalGetFactory(language); + return factory + ? { ...factory, getMetadata: () => { throw new Error('metadata exploded'); } } + : undefined; + }); + + const report = await diagnose([], deps); + + expect(report.languages).toHaveLength(1); + expect(report.languages[0].verdict).toBe('ok'); + expect(report.languages[0].modes?.attach.available).toBe(true); // from entry.attach + }); + it('merges collectExtras output into the reported details', async () => { const deps = makeDeps( [{ name: 'dotnet', validate: okValidate({ debuggerPath: '/opt/netcoredbg' }) }], diff --git a/tests/unit/cli/doctor/index.test.ts b/tests/unit/cli/doctor/index.test.ts index 38b6771e..c7873576 100644 --- a/tests/unit/cli/doctor/index.test.ts +++ b/tests/unit/cli/doctor/index.test.ts @@ -150,19 +150,22 @@ describe('handleDoctorCommand', () => { expect(deps.disposeLogger).toHaveBeenCalled(); }); - it('returns 2 for an unparseable --timeout', async () => { - const writeError = vi.fn(); - - const code = await handleDoctorCommand([], { timeout: 'soon' }, { - createDependencies: () => makeFakeDependencies([]), - writeOutput: vi.fn(), - writeError, - exit: vi.fn() - }); - - expect(code).toBe(2); - expect(writeError.mock.calls.join('\n')).toContain('timeout'); - }); + it.each(['soon', '1e4', '10s', '5,000', '-100', '0'])( + 'returns 2 for a non-integer --timeout %s instead of truncating it', + async (timeout) => { + const writeError = vi.fn(); + + const code = await handleDoctorCommand([], { timeout }, { + createDependencies: () => makeFakeDependencies([]), + writeOutput: vi.fn(), + writeError, + exit: vi.fn() + }); + + expect(code).toBe(2); + expect(writeError.mock.calls.join('\n')).toContain('timeout'); + } + ); it('force-exits only when a probe timed out (hung child containment)', async () => { const hungDeps = makeFakeDependencies([ diff --git a/tests/unit/cli/doctor/platform-checks.test.ts b/tests/unit/cli/doctor/platform-checks.test.ts index b51f533d..79590ed5 100644 --- a/tests/unit/cli/doctor/platform-checks.test.ts +++ b/tests/unit/cli/doctor/platform-checks.test.ts @@ -80,6 +80,31 @@ describe('checkContainerWorkspace', () => { expect(mount.status).toBe('skipped'); }); + it('warns when MCP_CONTAINER is set to a truthy-looking but unrecognized value', async () => { + const environment = makeEnvironment({ MCP_CONTAINER: '1' }); + const fileSystem = makeFileSystem(); + + const [containerMode, mount] = await checkContainerWorkspace(environment, fileSystem); + + expect(containerMode.status).toBe('warn'); + expect(containerMode.detail).toContain("'1'"); + expect(containerMode.fixHint).toContain('true'); + expect(mount.status).toBe('skipped'); + }); + + it('echoes the recognized MCP_CONTAINER value in the detail', async () => { + const environment = makeEnvironment({ MCP_CONTAINER: 'true', MCP_WORKSPACE_ROOT: '/workspace' }); + const fileSystem = makeFileSystem({ + stat: vi.fn().mockResolvedValue({ isDirectory: () => true }), + readdir: vi.fn().mockResolvedValue(['src']) + }); + + const [containerMode] = await checkContainerWorkspace(environment, fileSystem); + + expect(containerMode.status).toBe('ok'); + expect(containerMode.detail).toContain('MCP_CONTAINER=true'); + }); + it('reports broken when MCP_WORKSPACE_ROOT is unset in container mode', async () => { const environment = makeEnvironment({ MCP_CONTAINER: 'true' }); const fileSystem = makeFileSystem(); diff --git a/tests/unit/cli/doctor/presenters.test.ts b/tests/unit/cli/doctor/presenters.test.ts index d354bcb3..f16ca3c0 100644 --- a/tests/unit/cli/doctor/presenters.test.ts +++ b/tests/unit/cli/doctor/presenters.test.ts @@ -99,11 +99,26 @@ describe('presentLanguage', () => { expect(view.backend?.label).toContain('built-in'); }); - it('tolerates missing details entirely', () => { + it('returns empty cells when the probe produced no details (failed/timed-out probe)', () => { const view = presentLanguage('python', undefined); - expect(view.runtime).toMatchObject({ label: 'Python' }); - expect(view.runtime?.version).toBeUndefined(); + expect(view.runtime).toBeUndefined(); + expect(view.backend).toBeUndefined(); + }); + + it('omits a component that was not detected instead of naming it as if found', () => { + // dotnet with netcoredbg resolved but no SDK: the runtime cell must read + // as absent, not ".NET SDK". + const view = presentLanguage('dotnet', { debuggerPath: '/opt/netcoredbg/netcoredbg' }); + + expect(view.runtime).toBeUndefined(); + expect(view.backend).toMatchObject({ label: 'netcoredbg', path: '/opt/netcoredbg/netcoredbg' }); + }); + + it('keeps the vendored js-debug backend visible without a version (source counts as detection)', () => { + const view = presentLanguage('javascript', { nodeVersion: 'v22.0.0' }); + + expect(view.backend).toMatchObject({ label: 'js-debug', source: 'vendored' }); }); }); @@ -144,13 +159,15 @@ describe('collectDoctorExtras', () => { expect(extras).toEqual({ javacPath: '/opt/jdk/bin/javac' }); }); - it('collects the compiler version banner for cpp', async () => { - const importModule = vi.fn().mockResolvedValue({ - getCompilerInfo: vi.fn().mockResolvedValue({ command: 'g++', version: 'g++ (MinGW-w64) 13.2.0' }) - }); + it('collects the compiler version banner for cpp, reusing the already-discovered command', async () => { + const getCompilerInfo = vi.fn().mockResolvedValue({ command: 'g++', version: 'g++ (MinGW-w64) 13.2.0' }); + const importModule = vi.fn().mockResolvedValue({ getCompilerInfo }); const extras = await collectDoctorExtras('cpp', { compiler: 'g++' }, { importModule }); + // validate() already discovered the command — extras must not re-probe the + // whole candidate list. + expect(getCompilerInfo).toHaveBeenCalledWith('g++'); expect(extras).toEqual({ compilerVersion: 'g++ (MinGW-w64) 13.2.0' }); }); From 7a8ebaad5775e95cd45cc25197a6eb6a817a38ef Mon Sep 17 00:00:00 2001 From: JF Date: Sat, 22 Aug 2026 20:02:38 -0400 Subject: [PATCH 6/6] test(python): spawn fakes emit 'close' like real children (probe drain fix fallout) --- .../adapters/python/unit/python-utils.test.ts | 30 +++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/tests/adapters/python/unit/python-utils.test.ts b/tests/adapters/python/unit/python-utils.test.ts index 24e2b916..03ee1d30 100644 --- a/tests/adapters/python/unit/python-utils.test.ts +++ b/tests/adapters/python/unit/python-utils.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import { spawn } from 'child_process'; import fs from 'node:fs'; import path from 'node:path'; @@ -42,7 +42,7 @@ describe('python-utils', () => { proc.stderr = new EventEmitter(); // Default to successful validation - process.nextTick(() => proc.emit('exit', 0)); + process.nextTick(() => { proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -121,14 +121,14 @@ describe('python-utils', () => { if (hasDebugpyCallCount === 1) { process.nextTick(() => { proc.stdout.emit('data', Buffer.from('1.8.0')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); } else { - process.nextTick(() => proc.emit('exit', 1)); + process.nextTick(() => { proc.emit('exit', 1); proc.emit('close', 1); }); } } else { // Regular validation check - process.nextTick(() => proc.emit('exit', 0)); + process.nextTick(() => { proc.emit('exit', 0); proc.emit('close', 0); }); } return proc; @@ -222,7 +222,7 @@ describe('python-utils', () => { proc.stderr = new EventEmitter(); // Default to successful validation - process.nextTick(() => proc.emit('exit', 0)); + process.nextTick(() => { proc.emit('exit', 0); proc.emit('close', 0); }); return proc; }); @@ -260,14 +260,14 @@ describe('python-utils', () => { if (cmd === 'C:\\Windows\\py.exe') { process.nextTick(() => { proc.stdout.emit('data', Buffer.from('1.8.0')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); } else { - process.nextTick(() => proc.emit('exit', 1)); + process.nextTick(() => { proc.emit('exit', 1); proc.emit('close', 1); }); } } else { // Regular validation check - process.nextTick(() => proc.emit('exit', 0)); + process.nextTick(() => { proc.emit('exit', 0); proc.emit('close', 0); }); } return proc; @@ -298,10 +298,10 @@ describe('python-utils', () => { // Simulate Windows Store alias behavior process.nextTick(() => { proc.stderr.emit('data', Buffer.from('Python was not found; run without arguments to install from the Microsoft Store')); - proc.emit('exit', 9009); + proc.emit('exit', 9009); proc.emit('close', 9009); }); } else { - process.nextTick(() => proc.emit('exit', 0)); + process.nextTick(() => { proc.emit('exit', 0); proc.emit('close', 0); }); } return proc; @@ -327,7 +327,7 @@ describe('python-utils', () => { if (args?.[0] === '--version') { process.nextTick(() => { proc.stdout.emit('data', Buffer.from('Python 3.11.5\n')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); } @@ -347,7 +347,7 @@ describe('python-utils', () => { if (args?.[0] === '--version') { process.nextTick(() => { proc.stderr.emit('data', Buffer.from('Python 3.9.0')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); } @@ -376,7 +376,7 @@ describe('python-utils', () => { const proc = new EventEmitter() as any; proc.stdout = new EventEmitter(); proc.stderr = new EventEmitter(); - process.nextTick(() => proc.emit('exit', 1)); + process.nextTick(() => { proc.emit('exit', 1); proc.emit('close', 1); }); return proc; }); @@ -393,7 +393,7 @@ describe('python-utils', () => { if (args?.[0] === '--version') { process.nextTick(() => { proc.stdout.emit('data', Buffer.from('Custom Python Build')); - proc.emit('exit', 0); + proc.emit('exit', 0); proc.emit('close', 0); }); }