diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f66bb48..1de25537 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **`packageManager` pinned to pnpm@10.33.0** — a different pnpm major regenerated `pnpm-lock.yaml` incompatibly (one contributor diff silently dropped the entire security `overrides` block); the pin plus a CONTRIBUTING note make the wrong-pnpm case fail fast, and the workflows read the pin instead of duplicating a version (#478) ### Fixed +- **A typo'd attach `adapterConfig` key now gets a did-you-mean warning instead of silence** — every attach-capable adapter declares the attach keys its debugger actually consumes (`supportedAttachKeys`), and `attach_to_process` warns about keys outside that set with an edit-distance suggestion (`pathMapping (did you mean pathMappings?)`) while still **forwarding** them to the debug adapter untouched, so upstream debugger options the list doesn't model keep working with no mcp-debugger release; keys an adapter's transform genuinely drops keep warning as ignored (#450's contract). The javascript, ruby, java, and dotnet attach transforms also move from closed allowlists to the python/cpp deny-list pattern, so advanced attach options (js-debug's `localRoot`/`remoteRoot`/`sourceMaps`/`skipFiles`/`continueOnAttach`, rdbg's `localfsMap`, netcoredbg's `sourceFileMap`/`symbolOptions`) now genuinely reach the debugger. Thanks @abhijeetnardele24-hash for the `supportedAttachKeys` + did-you-mean design (#466) - **Java attach is inspectable on the first try** — the JDI bridge never suspends the VM on attach and java's policy lacked the post-attach pause every other attach adapter has, so the session reported `paused` while the JVM kept running: empty stack, "the debugger may not be paused", and no discoverable recovery. Java now pauses the whole VM after attach (the bridge anchors its stopped event to a thread that can actually report frames), the frameless-thread notes name a real recovery (`threadId` targeting or continue+pause), and `get_local_variables` no longer contradicts the session state (#465) - **`get_local_variables` walks down past an empty runtime top frame** — a pause inside a blocking syscall/sleep put a stdlib frame with no locals at `stackFrames[0]` and the tool returned an empty array even though the user frame's locals were one frame down and *already fetched*; extraction now re-anchors to the first frame that yields locals (zero extra DAP round-trips), reports that frame in `frame`, and a `note` discloses the walk-down. Skipped under an explicit `names:` filter (#468) - **Statement-anchored breakpoints survive `redefine_classes`** — the hot-swap replant was purely by line, so a line-shifting swap silently rebound a `statement:`-anchored breakpoint to whatever now lives at the old line, reporting `verified: true`; anchors are now re-resolved against the new source (the `restart_debugging` machinery) and re-sent after the redefine so the JDI replant binds the moved lines. The bridge's `replantedBreakpoints` count also no longer drops re-planted function breakpoints (#464) diff --git a/packages/adapter-cpp/src/cpp-debug-adapter.ts b/packages/adapter-cpp/src/cpp-debug-adapter.ts index d7e8b746..8251f575 100644 --- a/packages/adapter-cpp/src/cpp-debug-adapter.ts +++ b/packages/adapter-cpp/src/cpp-debug-adapter.ts @@ -103,6 +103,29 @@ export class CppDebugAdapter extends EventEmitter implements IDebugAdapter { readonly language = DebugLanguage.CPP; readonly name = 'C/C++ Debug Adapter'; + // CodeLLDB attach options (https://github.com/vadimcn/codelldb/blob/master/MANUAL.md) + // plus our processId sugar (normalized to pid). Unlisted keys still reach + // CodeLLDB (forwarded with a warning) — this list only powers recognition + + // typo suggestions (#466). + readonly supportedAttachKeys = [ + 'processId', + 'pid', + 'program', + 'stopOnEntry', + 'waitFor', + 'initCommands', + 'preRunCommands', + 'postRunCommands', + 'exitCommands', + 'targetCreateCommands', + 'processCreateCommands', + 'expressions', + 'sourceMap', + 'sourceLanguages', + 'relativePathBase', + 'breakpointMode' + ] as const; + private state: AdapterState = AdapterState.UNINITIALIZED; private dependencies: AdapterDependencies; private lastToolchainValidation: ToolchainValidationResult | undefined; diff --git a/packages/adapter-dotnet/src/DotnetDebugAdapter.ts b/packages/adapter-dotnet/src/DotnetDebugAdapter.ts index f67c799b..089fc181 100644 --- a/packages/adapter-dotnet/src/DotnetDebugAdapter.ts +++ b/packages/adapter-dotnet/src/DotnetDebugAdapter.ts @@ -106,6 +106,19 @@ export class DotnetDebugAdapter extends EventEmitter implements IDebugAdapter { readonly language = DebugLanguage.DOTNET; readonly name = '.NET Debug Adapter (netcoredbg)'; + // netcoredbg attach options plus the generic keys transformAttachConfig + // special-cases (sourcePaths feeds PDB discovery). Unlisted keys still reach + // netcoredbg (forwarded with a warning) — this list only powers recognition + // + typo suggestions (#466). + readonly supportedAttachKeys = [ + 'processId', + 'justMyCode', + 'stopOnEntry', + 'sourcePaths', + 'sourceFileMap', + 'symbolOptions' + ] as const; + private state: AdapterState = AdapterState.UNINITIALIZED; private dependencies: AdapterDependencies; @@ -405,23 +418,42 @@ export class DotnetDebugAdapter extends EventEmitter implements IDebugAdapter { } } - const attachConfig = { + const { + request: _request, + __attachMode: _attachMode, + host: _host, + port: _port, + processName: _processName, + identifierType: _identifierType, + ...rest + } = config as Record; + void _request; void _attachMode; void _host; void _port; + void _processName; void _identifierType; + + // Advanced passthrough with the normalized netcoredbg attach shape on top + // (issues #450/#466). The computed keys stay authoritative — in particular + // terminateDebuggee: false (never kill the debuggee on detach) must not be + // caller-overridable. + const attachConfig: Record = { + ...rest, type: 'coreclr', request: 'attach', name: '.NET: Attach', processId: config.processId ? Number(config.processId) : undefined, justMyCode: config.justMyCode ?? true, // CRITICAL: Never terminate the debuggee on detach - terminateDebuggee: false, - sourceFileMap: pdbScanDirs ? Object.fromEntries( - pdbScanDirs.map(p => [p, p]) - ) : undefined, - symbolOptions: symbolSearchPaths.length > 0 - ? { searchPaths: symbolSearchPaths, searchMicrosoftSymbolServer: false } - : undefined + terminateDebuggee: false }; + // Computed values win; a caller-provided sourceFileMap/symbolOptions from + // the spread survives when there is nothing computed to replace it. + if (pdbScanDirs) { + attachConfig.sourceFileMap = Object.fromEntries(pdbScanDirs.map(p => [p, p])); + } + if (symbolSearchPaths.length > 0) { + attachConfig.symbolOptions = { searchPaths: symbolSearchPaths, searchMicrosoftSymbolServer: false }; + } - return attachConfig; + return attachConfig as LanguageSpecificAttachConfig; } getDefaultAttachConfig(): Partial { diff --git a/packages/adapter-dotnet/tests/unit/dotnet-debug-adapter.test.ts b/packages/adapter-dotnet/tests/unit/dotnet-debug-adapter.test.ts index 0e622384..249e4a54 100644 --- a/packages/adapter-dotnet/tests/unit/dotnet-debug-adapter.test.ts +++ b/packages/adapter-dotnet/tests/unit/dotnet-debug-adapter.test.ts @@ -465,6 +465,32 @@ describe('DotnetDebugAdapter', () => { expect(defaults).toBeDefined(); expect(defaults!.justMyCode).toBe(true); }); + + it('forwards unknown attach keys but keeps terminateDebuggee non-overridable (issues #450/#466)', () => { + const result = adapter.transformAttachConfig({ + request: 'attach', + __attachMode: true, + processId: 1234, + terminateDebuggee: true, // must lose to the computed false + futureNetcoredbgOption: 'on' + } as never) as Record; + + expect(result.request).toBe('attach'); + expect(result.__attachMode).toBeUndefined(); + expect(result.futureNetcoredbgOption).toBe('on'); + expect(result.terminateDebuggee).toBe(false); + }); + + it('keeps a caller-provided sourceFileMap when nothing is computed', () => { + getProcessExecutableDirMock.mockReturnValue(null); + const result = adapter.transformAttachConfig({ + request: 'attach', + processId: 1234, + sourceFileMap: { '/remote': '/local' } + } as never) as Record; + + expect(result.sourceFileMap).toEqual({ '/remote': '/local' }); + }); }); // ===== Connection Management ===== diff --git a/packages/adapter-java/src/java-debug-adapter.ts b/packages/adapter-java/src/java-debug-adapter.ts index 1ca46736..181746f3 100644 --- a/packages/adapter-java/src/java-debug-adapter.ts +++ b/packages/adapter-java/src/java-debug-adapter.ts @@ -51,6 +51,21 @@ export class JavaDebugAdapter extends EventEmitter implements IDebugAdapter { readonly language = DebugLanguage.JAVA; readonly name = 'Java Debug Adapter (JDI)'; + // Keys the JDI bridge's attach handler reads (JdiDapServer.handleAttach) + // plus the generic keys transformAttachConfig special-cases. Unlisted keys + // still reach the bridge (forwarded with a warning) — this list only powers + // recognition + typo suggestions (#466). + readonly supportedAttachKeys = [ + 'host', + 'hostName', + 'port', + 'stopOnEntry', + 'timeout', + 'sourcePaths', + 'cwd', + 'env' + ] as const; + private state: AdapterState = AdapterState.UNINITIALIZED; private dependencies: AdapterDependencies; @@ -319,30 +334,28 @@ export class JavaDebugAdapter extends EventEmitter implements IDebugAdapter { } transformAttachConfig(config: GenericAttachConfig): LanguageSpecificAttachConfig { - const attachConfig: LanguageSpecificAttachConfig = { + const { + request: _request, + __attachMode: _attachMode, + processId: _processId, + processName: _processName, + identifierType: _identifierType, + host, + port, + ...rest + } = config as Record; + void _request; void _attachMode; void _processId; void _processName; + void _identifierType; + + // Advanced passthrough (the JDI bridge ignores keys it doesn't read) with + // the normalized attach shape on top (issues #450/#466). + return { + ...rest, type: 'java', request: 'attach', - host: config.host || 'localhost', - port: config.port, - }; - - if (config.sourcePaths) { - attachConfig.sourcePaths = config.sourcePaths; - } - if (config.stopOnEntry !== undefined) { - attachConfig.stopOnEntry = config.stopOnEntry; - } - if (config.cwd) { - attachConfig.cwd = config.cwd; - } - if (config.env) { - attachConfig.env = config.env; - } - if (config.timeout !== undefined) { - attachConfig.timeout = config.timeout; - } - - return attachConfig; + host: (host as string | undefined) || 'localhost', + port: port as number | undefined, + } as LanguageSpecificAttachConfig; } getDefaultAttachConfig(): Partial { diff --git a/packages/adapter-javascript/src/javascript-debug-adapter.ts b/packages/adapter-javascript/src/javascript-debug-adapter.ts index 439e276f..5e750fa5 100644 --- a/packages/adapter-javascript/src/javascript-debug-adapter.ts +++ b/packages/adapter-javascript/src/javascript-debug-adapter.ts @@ -41,6 +41,36 @@ export class JavascriptDebugAdapter extends EventEmitter implements IDebugAdapte readonly language = 'javascript' as unknown as DebugLanguage; readonly name = 'JavaScript/TypeScript Debug Adapter'; + // js-debug pwa-node attach options https://github.com/microsoft/vscode-js-debug/blob/main/package.json + // plus the generic keys transformAttachConfig special-cases. Unlisted keys + // still reach js-debug (forwarded with a warning) — this list only powers + // recognition + typo suggestions (#466). + readonly supportedAttachKeys = [ + 'host', + 'port', + 'address', + 'timeout', + 'localRoot', + 'remoteRoot', + 'smartStep', + 'skipFiles', + 'sourceMaps', + 'sourceMapPathOverrides', + 'outFiles', + 'outputCapture', + 'resolveSourceMapLocations', + 'pauseForSourceMap', + 'stopOnEntry', + 'justMyCode', + 'cwd', + 'env', + 'restart', + 'continueOnAttach', + 'trace', + 'websocketAddress', + 'attachExistingChildren' + ] as const; + private state: AdapterState = AdapterState.UNINITIALIZED; private readonly dependencies: AdapterDependencies; @@ -628,25 +658,29 @@ export class JavascriptDebugAdapter extends EventEmitter implements IDebugAdapte * mode and JsDebugAdapterPolicy.performHandshake sends a real DAP 'attach'. */ transformAttachConfig(config: GenericAttachConfig): LanguageSpecificAttachConfig { - const attachConfig: LanguageSpecificAttachConfig = { + const { + request: _request, + __attachMode: _attachMode, + processId: _processId, + processName: _processName, + identifierType: _identifierType, + host, + port, + ...rest + } = config as Record; + void _request; void _attachMode; void _processId; void _processName; + void _identifierType; + + // Advanced passthrough (localRoot/remoteRoot, sourceMaps, skipFiles, …) + // with the normalized pwa-node attach shape on top (issues #450/#466). + return { + ...rest, type: 'pwa-node', request: 'attach', name: 'Attach to Node.js process', - host: config.host || '127.0.0.1', - port: config.port, - }; - - if (config.stopOnEntry !== undefined) { - attachConfig.stopOnEntry = config.stopOnEntry; - } - if (config.justMyCode !== undefined) { - attachConfig.justMyCode = config.justMyCode; - } - if (config.timeout !== undefined) { - attachConfig.timeout = config.timeout; - } - - return attachConfig; + host: (host as string | undefined) || '127.0.0.1', + port: port as number | undefined, + } as LanguageSpecificAttachConfig; } getDefaultAttachConfig(): Partial { diff --git a/packages/adapter-javascript/tests/unit/javascript-debug-adapter.transform.test.ts b/packages/adapter-javascript/tests/unit/javascript-debug-adapter.transform.test.ts index b3c7f162..6fce1e02 100644 --- a/packages/adapter-javascript/tests/unit/javascript-debug-adapter.transform.test.ts +++ b/packages/adapter-javascript/tests/unit/javascript-debug-adapter.transform.test.ts @@ -330,4 +330,51 @@ describe('JavascriptDebugAdapter.transformLaunchConfig', () => { expect(env.NODE_OPTIONS ?? '').not.toContain('exitcode-shim'); }); }); + + describe('transformAttachConfig passthrough (issues #450/#466)', () => { + it('normalizes the pwa-node attach shape and defaults the host', () => { + const adapter = new JavascriptDebugAdapter(deps); + const cfg = adapter.transformAttachConfig({ + request: 'attach', + port: 9229, + stopOnEntry: true, + justMyCode: false, + timeout: 15000 + } as any) as Record; + + expect(cfg.type).toBe('pwa-node'); + expect(cfg.request).toBe('attach'); + expect(cfg.host).toBe('127.0.0.1'); + expect(cfg.port).toBe(9229); + expect(cfg.stopOnEntry).toBe(true); + expect(cfg.justMyCode).toBe(false); + expect(cfg.timeout).toBe(15000); + }); + + it('forwards advanced js-debug options and strips reserved keys', () => { + const adapter = new JavascriptDebugAdapter(deps); + const cfg = adapter.transformAttachConfig({ + request: 'launch', // must not survive: attach transforms pin the request + __attachMode: true, + processId: 4242, + host: '10.0.0.5', + port: 9229, + localRoot: '/local/src', + remoteRoot: '/app', + sourceMaps: false, + skipFiles: ['/**'], + continueOnAttach: true + } as any) as Record; + + expect(cfg.request).toBe('attach'); + expect(cfg.__attachMode).toBeUndefined(); + expect(cfg.processId).toBeUndefined(); + expect(cfg.host).toBe('10.0.0.5'); + expect(cfg.localRoot).toBe('/local/src'); + expect(cfg.remoteRoot).toBe('/app'); + expect(cfg.sourceMaps).toBe(false); + expect(cfg.skipFiles).toEqual(['/**']); + expect(cfg.continueOnAttach).toBe(true); + }); + }); }); diff --git a/packages/adapter-python/src/python-debug-adapter.ts b/packages/adapter-python/src/python-debug-adapter.ts index d3064502..11d7a856 100644 --- a/packages/adapter-python/src/python-debug-adapter.ts +++ b/packages/adapter-python/src/python-debug-adapter.ts @@ -84,6 +84,29 @@ export class PythonDebugAdapter extends EventEmitter implements IDebugAdapter { readonly language = DebugLanguage.PYTHON; readonly name = 'Python Debug Adapter'; + // debugpy attach schema https://github.com/microsoft/debugpy/wiki/Debug-configuration-settings + // plus the generic keys transformAttachConfig special-cases. Unlisted keys + // still reach debugpy (forwarded with a warning) — this list only powers + // recognition + typo suggestions (#466). + readonly supportedAttachKeys = [ + 'host', + 'port', + 'justMyCode', + 'pathMappings', + 'redirectOutput', + 'showReturnValue', + 'subProcess', + 'clientOS', + 'django', + 'jinja', + 'stopOnEntry', + 'cwd', + 'env', + 'logToFile', + 'steppingResumesAllThreads', + 'rules' + ] as const; + private state: AdapterState = AdapterState.UNINITIALIZED; private dependencies: AdapterDependencies; diff --git a/packages/adapter-ruby/src/ruby-debug-adapter.ts b/packages/adapter-ruby/src/ruby-debug-adapter.ts index 2f79986b..3476e78c 100644 --- a/packages/adapter-ruby/src/ruby-debug-adapter.ts +++ b/packages/adapter-ruby/src/ruby-debug-adapter.ts @@ -72,6 +72,21 @@ export class RubyDebugAdapter extends EventEmitter implements IDebugAdapter { readonly language = DebugLanguage.RUBY; readonly name = 'Ruby Debug Adapter (rdbg)'; + // Keys the rdbg attach path consumes (debug gem DAP) plus the generic keys + // transformAttachConfig special-cases. Unlisted keys still reach rdbg + // (forwarded with a warning) — this list only powers recognition + typo + // suggestions (#466). + readonly supportedAttachKeys = [ + 'host', + 'port', + 'stopOnEntry', + 'justMyCode', + 'cwd', + 'env', + 'localfs', + 'localfsMap' + ] as const; + private state: AdapterState = AdapterState.UNINITIALIZED; private dependencies: AdapterDependencies; private rubyPathCache = new Map(); @@ -376,7 +391,6 @@ export class RubyDebugAdapter extends EventEmitter implements IDebugAdapter { } transformAttachConfig(config: GenericAttachConfig): RubyAttachConfig { - const rawConfig = config as Record; const host = config.host || '127.0.0.1'; const port = config.port; @@ -387,7 +401,25 @@ export class RubyDebugAdapter extends EventEmitter implements IDebugAdapter { ); } + const { + request: _request, + __attachMode: _attachMode, + host: _host, + port: _port, + processId: _processId, + processName: _processName, + identifierType: _identifierType, + localfsMap, + ...rest + } = config as Record; + void _request; void _attachMode; void _host; void _port; + void _processId; void _processName; void _identifierType; + + // Advanced passthrough with the normalized rdbg attach shape on top + // (issues #450/#466); localfs stays computed from the host — localfsMap + // is the caller's path-mapping lever. const attachConfig: RubyAttachConfig = { + ...rest, type: 'rdbg', request: 'attach', name: 'Ruby: Attach', @@ -398,16 +430,8 @@ export class RubyDebugAdapter extends EventEmitter implements IDebugAdapter { justMyCode: config.justMyCode ?? true }; - if (typeof rawConfig.localfsMap === 'string') { - attachConfig.localfsMap = rawConfig.localfsMap; - } - - if (config.cwd) { - attachConfig.cwd = config.cwd; - } - - if (config.env) { - attachConfig.env = config.env; + if (typeof localfsMap === 'string') { + attachConfig.localfsMap = localfsMap; } return attachConfig; diff --git a/packages/adapter-ruby/tests/unit/ruby-debug-adapter.test.ts b/packages/adapter-ruby/tests/unit/ruby-debug-adapter.test.ts index 8d5dddcf..948d8f52 100644 --- a/packages/adapter-ruby/tests/unit/ruby-debug-adapter.test.ts +++ b/packages/adapter-ruby/tests/unit/ruby-debug-adapter.test.ts @@ -418,6 +418,25 @@ describe('RubyDebugAdapter', () => { .toThrow(AdapterError); }); + it('forwards unknown attach keys and strips reserved ones (issues #450/#466)', () => { + const adapter = new RubyDebugAdapter(createDependencies()); + const config = adapter.transformAttachConfig({ + request: 'attach', + __attachMode: true, + processId: 4242, + host: '127.0.0.1', + port: 12345, + nonstop: true + } as never) as Record; + + expect(config.request).toBe('attach'); + expect(config.__attachMode).toBeUndefined(); + expect(config.processId).toBeUndefined(); + expect(config.nonstop).toBe(true); + // Normalized keys stay authoritative + expect(config.localfs).toBe(true); + }); + it('tracks state and thread id from DAP events', () => { const adapter = new RubyDebugAdapter(createDependencies()); diff --git a/packages/shared/src/interfaces/adapter-policy-js.ts b/packages/shared/src/interfaces/adapter-policy-js.ts index 53172002..6a3b7225 100644 --- a/packages/shared/src/interfaces/adapter-policy-js.ts +++ b/packages/shared/src/interfaces/adapter-policy-js.ts @@ -41,9 +41,34 @@ export const JsDebugAdapterPolicy: AdapterPolicy = { childSessionStrategy: 'launchWithPendingTarget', buildChildStartArgs: (pendingId: string, parentConfig: Record) => { const type = typeof parentConfig?.type === 'string' ? (parentConfig.type as string) : 'pwa-node'; + // Carry the parent's forwardable attach extras (localRoot/remoteRoot, + // sourceMaps, skipFiles, …) into the child — source resolution happens + // here, so this is where they take effect (issue #466). The orchestration + // keys stay pinned/excluded: address/port/attachSimplePort would make the + // child a second direct attach to the same inspector (the #124 + // fight-over-the-process failure), and stopOnEntry is consumed by + // ChildSessionManager itself, not js-debug. + const { + request: _request, + name: _name, + __pendingTargetId: _pendingTargetId, + host: _host, + address: _address, + port: _port, + attachSimplePort: _attachSimplePort, + attachExistingChildren: _attachExistingChildren, + continueOnAttach: _continueOnAttach, + stopOnEntry: _stopOnEntry, + type: _type, + ...parentExtras + } = parentConfig ?? {}; + void _request; void _name; void _pendingTargetId; void _host; void _address; + void _port; void _attachSimplePort; void _attachExistingChildren; + void _continueOnAttach; void _stopOnEntry; void _type; return { command: 'attach', args: { + ...parentExtras, type, request: 'attach', __pendingTargetId: pendingId, @@ -392,7 +417,18 @@ export const JsDebugAdapterPolicy: AdapterPolicy = { // fight over the process — every pause is immediately resumed by the // other target and stackTrace fails with "Thread is not paused" // (observed empirically while fixing issue #124). + // Caller-provided attach extras (localRoot/remoteRoot, sourceMaps, + // skipFiles, …) are spread through so they reach js-debug — and, via the + // recorded attach args, its child sessions (issue #466). The policy's + // own keys stay on top: the #124 fight-over-the-process failure modes + // live exactly in these knobs. + const { + attachSimplePort: _ignoredSimplePort, + ...callerAttachExtras + } = baseRecord; + void _ignoredSimplePort; const attachArgs: Record = { + ...callerAttachExtras, type, request: 'attach', address: attachHost, diff --git a/packages/shared/src/interfaces/debug-adapter.ts b/packages/shared/src/interfaces/debug-adapter.ts index 7ca07e77..3a58f317 100644 --- a/packages/shared/src/interfaces/debug-adapter.ts +++ b/packages/shared/src/interfaces/debug-adapter.ts @@ -148,6 +148,21 @@ export interface IDebugAdapter extends EventEmitter { */ usesDirectConnectForAttach?(): boolean; + /** + * The adapterConfig keys this adapter's debugger/bridge is known to consume + * on attach. When declared, the session layer warns about caller-provided + * keys outside this list — with an edit-distance typo suggestion (e.g. + * pathMapping → "did you mean pathMappings?") — but still FORWARDS them to + * the debug adapter untouched (issue #466), so upstream debugger options the + * list doesn't model remain usable. Keys the attach transform itself drops + * are reported separately as ignored (issue #450). + * + * Ground the list in what actually does something: the upstream debugger's + * attach schema plus keys transformAttachConfig special-cases. If omitted, + * only the transform-drop warning applies. + */ + readonly supportedAttachKeys?: readonly string[]; + /** * Transform generic attach config to language-specific format * Only called if supportsAttach() returns true @@ -156,7 +171,10 @@ export interface IDebugAdapter extends EventEmitter { * body, so prefer a deny-list (strip known-bad keys, spread the rest) over * an allowlist: unknown adapterConfig keys the caller provided should reach * the debug adapter untouched. Caller-provided adapterConfig keys missing - * from the result are surfaced as a warning by the session layer (#450). + * from the result are surfaced as a warning by the session layer (#450); + * kept keys outside `supportedAttachKeys` warn as forwarded-unrecognized + * with a typo suggestion (#466). + * * @param config Generic attach configuration * @returns Language-specific attach configuration */ diff --git a/src/proxy/minimal-dap.ts b/src/proxy/minimal-dap.ts index 17aabcdf..d382d2ac 100644 --- a/src/proxy/minimal-dap.ts +++ b/src/proxy/minimal-dap.ts @@ -397,8 +397,12 @@ export class MinimalDapClient extends EventEmitter { * reverse startDebugging configuration only carries * {type, name, __pendingTargetId}: without this, ChildSessionManager cannot * distinguish attach-mode children from launch-mode children, nor see - * whether the user asked for an entry stop (issue #124). Launch-mode - * configs are returned unchanged. + * whether the user asked for an entry stop (issue #124). Caller-provided + * attach extras (localRoot/remoteRoot, sourceMaps, skipFiles, …) ride along + * too — the child session is where source resolution actually happens, so + * dropping them here would make forwarded attach options inert (issue #466). + * js-debug's own child keys win over the parent's. Launch-mode configs are + * returned unchanged. */ private enrichChildConfig(config: ChildSessionConfig): ChildSessionConfig { const start = this.lastStartRequestArgs; @@ -406,6 +410,7 @@ export class MinimalDapClient extends EventEmitter { return config; } const parentConfig: Record = { + ...start, ...(config.parentConfig ?? {}), request: 'attach' }; diff --git a/src/server.ts b/src/server.ts index 483b9757..1fbb710c 100644 --- a/src/server.ts +++ b/src/server.ts @@ -1191,7 +1191,7 @@ export class DebugMcpServer { stopOnEntry: { type: 'boolean', description: 'Stop on entry after attaching' }, justMyCode: { type: 'boolean', description: 'Only debug user code (skip library code)' }, breakOnExceptions: { type: 'string', enum: ['uncaught', 'all', 'none'], description: 'Break when exceptions are thrown: "uncaught" pauses at uncaught exceptions at the crash site; "all" also pauses on caught/raised exceptions (language-dependent). Default "none" — attach sessions never apply a language default (unlike launch)' }, - adapterConfig: { type: 'object', description: 'Adapter-specific attach configuration merged into the attach config before the adapter transforms it (e.g. C/C++/LLDB: program — the binary path for symbol resolution when /proc//maps paths are not openable, as in a kubectl-debug ephemeral container — or initCommands like "settings set target.exec-search-paths /proc//root"). Reserved keys request/__attachMode are ignored; set stopOnEntry via the top-level parameter. Not applied to js-debug attach, which builds its own attach request', additionalProperties: true } + adapterConfig: { type: 'object', description: 'Adapter-specific attach configuration merged into the attach config before the adapter transforms it (e.g. C/C++/LLDB: program — the binary path for symbol resolution when /proc//maps paths are not openable, as in a kubectl-debug ephemeral container — or initCommands like "settings set target.exec-search-paths /proc//root"). Reserved keys request/__attachMode are ignored; set stopOnEntry via the top-level parameter. Keys the adapter does not recognize are still forwarded to the debugger and named in the response warning — a near-miss of a supported key gets a did-you-mean suggestion. js-debug pins its attach orchestration keys (address/port/continueOnAttach/attachExistingChildren) over caller values', additionalProperties: true } }, required: ['sessionId'] } diff --git a/src/session/session-manager-operations.ts b/src/session/session-manager-operations.ts index 7961ae63..89764dfc 100644 --- a/src/session/session-manager-operations.ts +++ b/src/session/session-manager-operations.ts @@ -25,6 +25,7 @@ import { ProxyConfig } from '../proxy/proxy-config.js'; import { MIRROR_EXPOSE_COMMAND, MIRROR_UNEXPOSE_COMMAND } from '../proxy/dap-proxy-interfaces.js'; import { ErrorMessages } from '../utils/error-messages.js'; import { checkLaunchToolchain } from '../utils/language-availability.js'; +import { didYouMean } from '../utils/did-you-mean.js'; import { resolveStatement } from '../utils/breakpoint-resolver.js'; import { normalizeBreakpointMessage } from '../utils/breakpoint-message.js'; import { SessionManagerData } from './session-manager-data.js'; @@ -296,19 +297,44 @@ export abstract class SessionManagerOperations extends SessionManagerData { transformedLaunchConfig = undefined; } - // Attach transforms may be allowlists that silently discard adapterConfig - // keys (issue #450) — record the drops so attachToProcess can warn the - // caller. Assigned unconditionally so a prior attach's record never leaks. + // Attach transforms may strip adapterConfig keys (issue #450) — record the + // drops so attachToProcess can warn the caller. Keys the transform kept but + // the adapter doesn't declare in supportedAttachKeys are forwarded to the + // debug adapter as-is, and recorded separately so the caller learns they + // weren't recognized (issue #466) — never deleted, so upstream debugger + // capabilities stay reachable without an mcp-debugger release. Both records + // are assigned unconditionally so a prior attach's record never leaks. if (isAttachMode) { - const dropped = transformedLaunchConfig - ? adapterExtraKeys.filter((key) => !(key in transformedLaunchConfig!)) - : []; + const supportedKeys = adapter.supportedAttachKeys; + // A typo of a supported key is the likeliest caller mistake — annotate + // both buckets with an edit-distance suggestion when a list is declared. + const describeKey = (key: string): string => { + const suggestion = supportedKeys ? didYouMean(key, supportedKeys) : null; + return suggestion ? `${key} (did you mean ${suggestion}?)` : key; + }; + + const dropped: string[] = []; + const forwardedUnknown: string[] = []; + for (const key of adapterExtraKeys) { + if (transformedLaunchConfig && !(key in transformedLaunchConfig)) { + dropped.push(describeKey(key)); + } else if (supportedKeys && !supportedKeys.includes(key)) { + forwardedUnknown.push(describeKey(key)); + } + } + if (dropped.length > 0) { this.logger.warn( `[SessionManager] ${session.language} attach transform dropped adapterConfig key(s) for session ${sessionId}: ${dropped.join(', ')}` ); } + if (forwardedUnknown.length > 0) { + this.logger.warn( + `[SessionManager] ${session.language} attach forwarded unrecognized adapterConfig key(s) for session ${sessionId}: ${forwardedUnknown.join(', ')}` + ); + } session.attachDroppedConfigKeys = dropped.length > 0 ? dropped : undefined; + session.attachForwardedUnknownConfigKeys = forwardedUnknown.length > 0 ? forwardedUnknown : undefined; } const adapterWithToolchain = adapter as { @@ -2908,12 +2934,25 @@ export abstract class SessionManagerOperations extends SessionManagerData { attachConfig }; // Surface adapterConfig keys the adapter's attach transform dropped - // (issue #450) — "unknown attach keys should either work or warn". + // (issue #450) and keys forwarded to the adapter unrecognized (issue + // #466) — "unknown attach keys should either work or warn". const droppedKeys = session.attachDroppedConfigKeys; + const forwardedKeys = session.attachForwardedUnknownConfigKeys; + session.attachDroppedConfigKeys = undefined; + session.attachForwardedUnknownConfigKeys = undefined; + const warningParts: string[] = []; if (droppedKeys && droppedKeys.length > 0) { - session.attachDroppedConfigKeys = undefined; - attachData.warning = - `adapterConfig key(s) not supported by the ${session.language} attach request were ignored: ${droppedKeys.join(', ')}`; + warningParts.push( + `adapterConfig key(s) not supported by the ${session.language} attach request were ignored: ${droppedKeys.join(', ')}` + ); + } + if (forwardedKeys && forwardedKeys.length > 0) { + warningParts.push( + `adapterConfig key(s) not recognized by mcp-debugger were forwarded to the ${session.language} adapter as-is: ${forwardedKeys.join(', ')}` + ); + } + if (warningParts.length > 0) { + attachData.warning = warningParts.join('; '); } return { diff --git a/src/session/session-store.ts b/src/session/session-store.ts index 42649556..676054ea 100644 --- a/src/session/session-store.ts +++ b/src/session/session-store.ts @@ -118,6 +118,10 @@ export interface ManagedSession extends DebugSessionInfo { // carry into the DAP attach request (issue #450). Recorded per attach by // startProxyManager; consumed by attachToProcess for the response warning. attachDroppedConfigKeys?: string[]; + // Caller-provided adapterConfig keys outside the adapter's declared + // supportedAttachKeys that were still forwarded to the debug adapter + // (issue #466). Same lifecycle as attachDroppedConfigKeys. + attachForwardedUnknownConfigKeys?: string[]; // The most recent real launch, replayed by restart_debugging (issue #238). lastLaunch?: LastLaunchSpec; // Live DAP mirror endpoint, when exposed via expose_session (issue #217). diff --git a/src/utils/did-you-mean.ts b/src/utils/did-you-mean.ts new file mode 100644 index 00000000..5388d10d --- /dev/null +++ b/src/utils/did-you-mean.ts @@ -0,0 +1,58 @@ +/** + * Calculates the Levenshtein distance between two strings. + */ +function levenshtein(a: string, b: string): number { + if (a.length === 0) return b.length; + if (b.length === 0) return a.length; + + const matrix = Array.from({ length: a.length + 1 }, () => new Array(b.length + 1).fill(0)); + + for (let i = 0; i <= a.length; i++) { + matrix[i][0] = i; + } + for (let j = 0; j <= b.length; j++) { + matrix[0][j] = j; + } + + for (let i = 1; i <= a.length; i++) { + for (let j = 1; j <= b.length; j++) { + const cost = a[i - 1] === b[j - 1] ? 0 : 1; + matrix[i][j] = Math.min( + matrix[i - 1][j] + 1, // deletion + matrix[i][j - 1] + 1, // insertion + matrix[i - 1][j - 1] + cost // substitution + ); + } + } + + return matrix[a.length][b.length]; +} + +/** + * Finds the closest matching string from a list of valid strings using Levenshtein distance. + * Returns the closest match if its distance is within the threshold, otherwise returns null. + * + * Real typos are almost always 1-2 edits; distance 3 starts reaching *different* + * keys (a wrong suggestion an agent will obey), so the default threshold is 2. + * For short strings (<= 4 characters) the threshold is strictly 1 — e.g. `host` + * must not suggest `port`. + */ +export function didYouMean(target: string, validStrings: readonly string[], threshold = 2): string | null { + if (!validStrings || validStrings.length === 0) return null; + + const actualThreshold = target.length <= 4 ? 1 : threshold; + + let closestMatch: string | null = null; + let minDistance = Infinity; + + for (const validString of validStrings) { + const distance = levenshtein(target.toLowerCase(), validString.toLowerCase()); + + if (distance <= actualThreshold && distance < minDistance) { + minDistance = distance; + closestMatch = validString; + } + } + + return closestMatch; +} diff --git a/tests/adapters/java/unit/java-debug-adapter.test.ts b/tests/adapters/java/unit/java-debug-adapter.test.ts index b8c456fc..f180937a 100644 --- a/tests/adapters/java/unit/java-debug-adapter.test.ts +++ b/tests/adapters/java/unit/java-debug-adapter.test.ts @@ -687,6 +687,21 @@ describe('JavaDebugAdapter', () => { // No mandatory timeout — JDI bridge doesn't require it expect(config.timeout).toBeUndefined(); }); + + it('forwards unknown adapterConfig keys and strips reserved ones (issues #450/#466)', () => { + const config = adapter.transformAttachConfig({ + request: 'launch', // must not survive: attach transforms pin the request + __attachMode: true, + processId: 4242, + port: 5005, + futureJdiOption: 'yes', + } as never); + + expect(config.request).toBe('attach'); + expect((config as Record).__attachMode).toBeUndefined(); + expect((config as Record).processId).toBeUndefined(); + expect((config as Record).futureJdiOption).toBe('yes'); + }); }); describe('getDefaultAttachConfig', () => { diff --git a/tests/core/unit/server/server-redefine-and-attach.test.ts b/tests/core/unit/server/server-redefine-and-attach.test.ts index 5bdf8830..497abcd6 100644 --- a/tests/core/unit/server/server-redefine-and-attach.test.ts +++ b/tests/core/unit/server/server-redefine-and-attach.test.ts @@ -438,7 +438,7 @@ describe('redefine_classes and attach stopOnEntry tests', () => { state: 'paused', data: { message: 'Attached to process at 127.0.0.1:5678', - warning: 'adapterConfig key(s) not supported by the python attach request were ignored: localRoot, remoteRoot', + warning: 'adapterConfig key(s) not supported by the python attach request were ignored: remoteRoot; adapterConfig key(s) not recognized by mcp-debugger were forwarded to the python adapter as-is: pathMapping (did you mean pathMappings?)', }, }); @@ -452,7 +452,8 @@ describe('redefine_classes and attach stopOnEntry tests', () => { const payload = JSON.parse(result.content[0].text); expect(payload.success).toBe(true); - expect(payload.warning).toContain('localRoot, remoteRoot'); + expect(payload.warning).toContain('were ignored: remoteRoot'); + expect(payload.warning).toContain('forwarded to the python adapter as-is: pathMapping (did you mean pathMappings?)'); }); it('adds no top-level warning when the attach succeeded cleanly', async () => { diff --git a/tests/core/unit/session/session-manager-attach-modes.test.ts b/tests/core/unit/session/session-manager-attach-modes.test.ts index 02e02a11..22aceb24 100644 --- a/tests/core/unit/session/session-manager-attach-modes.test.ts +++ b/tests/core/unit/session/session-manager-attach-modes.test.ts @@ -293,14 +293,18 @@ describe('SessionManagerOperations attach modes', () => { }); describe('dropped adapterConfig keys warning (issue #450)', () => { - function makeDirectConnectAdapter(transform: (cfg: unknown) => unknown) { + function makeDirectConnectAdapter( + transform: (cfg: unknown) => unknown, + supportedAttachKeys?: readonly string[] + ) { return { resolveExecutablePath: vi.fn(), buildAdapterCommand: vi.fn(), usesDirectConnectForAttach: vi.fn().mockReturnValue(true), supportsAttach: vi.fn().mockReturnValue(true), transformAttachConfig: vi.fn().mockImplementation(transform), - getDefaultExecutableName: vi.fn().mockReturnValue('ruby') + getDefaultExecutableName: vi.fn().mockReturnValue('ruby'), + ...(supportedAttachKeys ? { supportedAttachKeys } : {}) }; } @@ -368,6 +372,77 @@ describe('SessionManagerOperations attach modes', () => { expect((result.data as { warning?: string }).warning).toBeUndefined(); }); + it('forwards keys outside supportedAttachKeys with a did-you-mean warning (issue #466)', async () => { + const adapterStub = makeDirectConnectAdapter( + (cfg) => cfg, + ['pathMappings', 'justMyCode'] + ); + mockDependencies.adapterRegistry.create.mockResolvedValue(adapterStub); + + const result = await operations.attachToProcess('test-session', { + host: '127.0.0.1', + port: 12345, + stopOnEntry: false, + adapterConfig: { pathMapping: [{ localRoot: 'C:\\x', remoteRoot: '/app' }] } + }); + + expect(result.success).toBe(true); + const warning = (result.data as { warning?: string }).warning; + expect(warning).toContain('not recognized by mcp-debugger were forwarded to the ruby adapter as-is'); + expect(warning).toContain('pathMapping (did you mean pathMappings?)'); + expect(warning).not.toContain('were ignored'); + + // Forwarded means forwarded: the typo'd key must still reach the DAP + // attach config handed to the proxy. + const proxyConfig = mockProxyManager.start.mock.calls[0][0]; + expect(proxyConfig.launchConfig).toHaveProperty('pathMapping'); + }); + + it('still warns "ignored" for a listed key the transform drops (union with #450)', async () => { + const adapterStub = makeDirectConnectAdapter( + (cfg) => ({ request: 'attach', keepMe: (cfg as Record).keepMe }), + ['keepMe', 'alsoSupported'] + ); + mockDependencies.adapterRegistry.create.mockResolvedValue(adapterStub); + + const result = await operations.attachToProcess('test-session', { + host: '127.0.0.1', + port: 12345, + stopOnEntry: false, + adapterConfig: { keepMe: 1, alsoSupported: 2 } + }); + + expect(result.success).toBe(true); + const warning = (result.data as { warning?: string }).warning; + expect(warning).toContain('were ignored: alsoSupported'); + expect(warning).not.toContain('keepMe'); + }); + + it('reports dropped and forwarded-unrecognized keys in one combined warning', async () => { + const adapterStub = makeDirectConnectAdapter( + (cfg) => { + const c = cfg as Record; + return { request: 'attach', keepMe: c.keepMe, mystery: c.mystery }; + }, + ['keepMe'] + ); + mockDependencies.adapterRegistry.create.mockResolvedValue(adapterStub); + + const result = await operations.attachToProcess('test-session', { + host: '127.0.0.1', + port: 12345, + stopOnEntry: false, + adapterConfig: { keepMe: 1, mystery: 2, localRoot: 'C:\\x' } + }); + + expect(result.success).toBe(true); + const warning = (result.data as { warning?: string }).warning ?? ''; + expect(warning).toContain('were ignored: localRoot'); + expect(warning).toContain('forwarded to the ruby adapter as-is: mystery'); + expect(warning.indexOf('; ')).toBeGreaterThan(0); + expect(warning).not.toContain('keepMe'); + }); + it('does not leak a stale warning into a later attach on the same session', async () => { const dropAll = makeDirectConnectAdapter(() => ({ request: 'attach' })); mockDependencies.adapterRegistry.create.mockResolvedValue(dropAll); diff --git a/tests/core/unit/utils/did-you-mean.test.ts b/tests/core/unit/utils/did-you-mean.test.ts new file mode 100644 index 00000000..42dff796 --- /dev/null +++ b/tests/core/unit/utils/did-you-mean.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect } from 'vitest'; +import { didYouMean } from '../../../../src/utils/did-you-mean.js'; + +describe('didYouMean', () => { + const validStrings = [ + 'pathMappings', + 'justMyCode', + 'stopOnEntry', + 'args', + 'cwd', + 'env', + 'envFile' + ]; + + it('finds exact matches (distance 0)', () => { + expect(didYouMean('pathMappings', validStrings)).toBe('pathMappings'); + expect(didYouMean('cwd', validStrings)).toBe('cwd'); + }); + + it('suggests fixes for 1-edit typos', () => { + // Missing character + expect(didYouMean('pathMapping', validStrings)).toBe('pathMappings'); + // Extra character + expect(didYouMean('justMyCodes', validStrings)).toBe('justMyCode'); + // Substituted character + expect(didYouMean('stopOnEntri', validStrings)).toBe('stopOnEntry'); + // Case difference + expect(didYouMean('pathmappings', validStrings)).toBe('pathMappings'); + }); + + it('suggests fixes for 2-edit typos on normal-length keys', () => { + // 'justMyCo' -> 'justMyCode' is 2 insertions + expect(didYouMean('justMyCo', validStrings)).toBe('justMyCode'); + }); + + it('returns null if no matches within threshold', () => { + expect(didYouMean('completelyWrongKey', validStrings)).toBeNull(); + // Distance 3 is where misleading suggestions live — 'justMyC' is 3 edits + // from 'justMyCode' and must not suggest it + expect(didYouMean('justMyC', validStrings)).toBeNull(); + }); + + it('applies stricter threshold for short strings', () => { + // For length <= 4, threshold is 1 + // 'cwe' distance from 'cwd' is 1 -> match + expect(didYouMean('cwe', validStrings)).toBe('cwd'); + + // 'cw' distance from 'cwd' is 1 -> match + expect(didYouMean('cw', validStrings)).toBe('cwd'); + + // 'cx' distance from 'cwd' is 2 -> should return null because short strings have threshold 1 + expect(didYouMean('cx', validStrings)).toBeNull(); + + // 'host' is a valid concept on other adapters, 2 edits from 'port' — + // must never be suggested as a typo of it + expect(didYouMean('host', ['port', 'address'])).toBeNull(); + }); + + it('handles empty valid strings gracefully', () => { + expect(didYouMean('test', [])).toBeNull(); + }); +});