From 943d9b06c019dc6de926cc5b5a1a8657663e104b Mon Sep 17 00:00:00 2001 From: AlexKovynev Date: Wed, 9 Sep 2026 13:09:07 +0300 Subject: [PATCH] Fix Ruby variable inspection --- CHANGELOG.md | 7 +- docs/architecture/debuggingExecutor.md | 4 + docs/architecture/debuggingHandler.md | 4 + src/debuggingExecutor.ts | 31 +++- src/debuggingHandler.ts | 39 ++++- src/test/mixedVariableChildren.test.ts | 62 ++++++++ src/test/rubyInspection.test.ts | 199 +++++++++++++++++++++++++ 7 files changed, 334 insertions(+), 12 deletions(-) create mode 100644 src/test/mixedVariableChildren.test.ts create mode 100644 src/test/rubyInspection.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cc55e09..242d4ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,11 @@ All notable changes to DebugMCP will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/), and this project adheres to [Semantic Versioning](https://semver.org/). +## [Unreleased] + +### Fixed +- Preserve Ruby scalar values and retrieve both indexed and named children, with Ruby metadata filtering scoped to Ruby LSP. + ## [2.3.4] - 2026-09-03 ### Added @@ -117,4 +122,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this - Initial release - Core debugging capabilities via MCP protocol - VS Code Debug Adapter Protocol integration -- Automatic MCP server startup on extension activation \ No newline at end of file +- Automatic MCP server startup on extension activation diff --git a/docs/architecture/debuggingExecutor.md b/docs/architecture/debuggingExecutor.md index d2aaea0..b2f23ab 100644 --- a/docs/architecture/debuggingExecutor.md +++ b/docs/architecture/debuggingExecutor.md @@ -113,3 +113,7 @@ For `coreclr` debug type, the executor uses a different approach: - Executes `testing.debugCurrentFile` command This handles .NET's test debugging workflow which differs from other languages. + +## Variable inspection + +When a parent advertises indexed children, retrieve both indexed and named groups, including adapters that omit the named count. This preserves custom properties on containers. Parents without indexed children retain the unfiltered variables request. diff --git a/docs/architecture/debuggingHandler.md b/docs/architecture/debuggingHandler.md index 4c737b5..b3bb719 100644 --- a/docs/architecture/debuggingHandler.md +++ b/docs/architecture/debuggingHandler.md @@ -105,3 +105,7 @@ Recursive expansion is bounded to 100 child fields total per response, shared ac All operations wrap errors with context about what operation failed, enabling AI agents to understand and potentially recover from failures. Expression evaluation also distinguishes an adapter error from a successful command whose result/output was not captured. + +## Variable inspection + +For `ruby_lsp` sessions, Ruby scalar values keep their result even when rdbg attaches metadata children. Synthetic `#class` and `%ancestors` children are omitted only for Ruby. Existing secret redaction and names/types-only descendant rendering remain in force. diff --git a/src/debuggingExecutor.ts b/src/debuggingExecutor.ts index 5ec5333..43c7939 100644 --- a/src/debuggingExecutor.ts +++ b/src/debuggingExecutor.ts @@ -23,6 +23,10 @@ export interface TestDebugDispatch { /** * Interface for debugging execution operations */ +export interface VariableChildrenOptions { + indexedVariables?: number; +} + export interface IDebuggingExecutor { startDebugging(workingDirectory: string, config: string | vscode.DebugConfiguration): Promise; debugTestAtCursor(fileFullPath: string, testName: string): Promise; @@ -37,7 +41,7 @@ export interface IDebuggingExecutor { removeBreakpoint(uri: vscode.Uri, line: number): Promise; getCurrentDebugState(numNextLines: number): Promise; getVariables(frameId: number, scope?: 'local' | 'global' | 'all'): Promise; - getVariableChildren(variablesReference: number): Promise; + getVariableChildren(variablesReference: number, options?: VariableChildrenOptions): Promise; evaluateExpression(expression: string, frameId: number): Promise; getBreakpoints(): readonly vscode.Breakpoint[]; clearAllBreakpoints(): void; @@ -551,7 +555,10 @@ export class DebuggingExecutor implements IDebuggingExecutor { * handler only reads children for variables explicitly requested by the * caller, rather than recursively dumping every value in scope. */ - public async getVariableChildren(variablesReference: number): Promise { + public async getVariableChildren( + variablesReference: number, + options: VariableChildrenOptions = {} + ): Promise { if (variablesReference <= 0) { return []; } @@ -562,9 +569,23 @@ export class DebuggingExecutor implements IDebuggingExecutor { throw new Error('No active debug session'); } - const response = await this.dapRequest(activeSession, 'variables', { - variablesReference - }); + const indexedVariables = Number(options.indexedVariables) || 0; + if (indexedVariables > 0) { + const indexed = await this.dapRequest(activeSession, 'variables', { + variablesReference, + filter: 'indexed', + start: 0, + count: indexedVariables + }); + // The named count is optional. Always request this group so + // containers with custom properties retain those children too. + const named = await this.dapRequest(activeSession, 'variables', { + variablesReference, + filter: 'named' + }); + return [ ...(indexed?.variables ?? []), ...(named?.variables ?? []) ]; + } + const response = await this.dapRequest(activeSession, 'variables', { variablesReference }); return response?.variables || []; } catch (error) { throw new Error(`Failed to expand variable: ${error}`); diff --git a/src/debuggingHandler.ts b/src/debuggingHandler.ts index 12fdc69..0377854 100644 --- a/src/debuggingHandler.ts +++ b/src/debuggingHandler.ts @@ -684,6 +684,26 @@ export class DebuggingHandler implements IDebuggingHandler { return /[*&]\s*$/.test(type.split(/\r?\n/, 1)[0].trim()); } + /** + * Some adapters expose implementation metadata as children of scalar + * values. Ruby rdbg, for example, gives Integer and String values a + * variablesReference for #class and other internals. Those references do + * not make the user value an aggregate and should not hide its result. + */ + private isScalarLikeType(type: unknown): boolean { + if (this.executor.getActiveSession?.()?.type.toLowerCase() !== 'ruby_lsp' || typeof type !== 'string') { + return false; + } + + const typeName = type.split(/\r?\n/, 1)[0].trim(); + return /^(?:Integer|Float|Rational|Complex|String|Symbol|TrueClass|FalseClass|NilClass|Regexp)$/.test(typeName); + } + + private isAdapterMetadataVariable(variable: any): boolean { + return this.executor.getActiveSession?.()?.type.toLowerCase() === 'ruby_lsp' && + (variable?.name === '#class' || variable?.name === '%ancestors'); + } + /** * List the variable names (and types) visible at the current execution * point, deliberately without any values, so an agent can discover what @@ -824,7 +844,8 @@ export class DebuggingHandler implements IDebuggingHandler { if (response && response.result !== undefined) { let resultText = `Expression: ${expression}\n`; const isComplex = response.variablesReference > 0 && - !DebuggingHandler.isPointerLikeType(response.type); + !DebuggingHandler.isPointerLikeType(response.type) && + !this.isScalarLikeType(response.type); const expressionIsSensitive = isSensitiveExpression(expression); const { value, redacted } = isComplex ? { @@ -842,7 +863,8 @@ export class DebuggingHandler implements IDebuggingHandler { ' ', 1, new Set(), - { remaining: this.maxExpandedFields } + { remaining: this.maxExpandedFields }, + response ); if (children.text) { resultText += `\n${children.text}`; @@ -891,7 +913,8 @@ export class DebuggingHandler implements IDebuggingHandler { let redacted = false; const variablesReference = Number(variable.variablesReference) || 0; const isComplex = variablesReference > 0 && - !DebuggingHandler.isPointerLikeType(variable.type); + !DebuggingHandler.isPointerLikeType(variable.type) && + !this.isScalarLikeType(variable.type); if (includeValue && isComplex) { const redactionName = DebuggingHandler.redactionVariableName(variable, name); if (isSensitiveName(redactionName)) { @@ -915,7 +938,8 @@ export class DebuggingHandler implements IDebuggingHandler { `${indent} `, depth + 1, visitedReferences, - expansionBudget + expansionBudget, + variable ); if (children.text) { text += `\n${children.text}`; @@ -931,7 +955,8 @@ export class DebuggingHandler implements IDebuggingHandler { indent: string, depth: number, visitedReferences: Set, - expansionBudget: { remaining: number } + expansionBudget: { remaining: number }, + parent: any = {} ): Promise<{ text: string; redacted: boolean }> { if (depth > this.maxVariableExpansionDepth) { return { text: `${indent}`, redacted: false }; @@ -942,7 +967,9 @@ export class DebuggingHandler implements IDebuggingHandler { const nextVisited = new Set(visitedReferences); nextVisited.add(variablesReference); - const children = await this.executor.getVariableChildren(variablesReference); + const children = (await this.executor.getVariableChildren(variablesReference, { + indexedVariables: parent.indexedVariables + })).filter(child => !this.isAdapterMetadataVariable(child)); const rendered: string[] = []; let redacted = false; let renderedChildren = 0; diff --git a/src/test/mixedVariableChildren.test.ts b/src/test/mixedVariableChildren.test.ts new file mode 100644 index 0000000..aeb4949 --- /dev/null +++ b/src/test/mixedVariableChildren.test.ts @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. + +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { DebuggingExecutor, IDebuggingExecutor } from '../debuggingExecutor'; +import { DebuggingHandler } from '../debuggingHandler'; + +suite('Mixed indexed and named variable children', () => { + for (const type of [ 'ruby_lsp', 'pwa-node', 'cppdbg' ]) { + test(`${type}: retrieves indexed elements and named properties`, async () => { + const descriptor = Object.getOwnPropertyDescriptor(vscode.debug, 'activeDebugSession')!; + const requests: unknown[] = []; + const indexed = { name: '0', type: 'Integer', variablesReference: 0 }; + const named = { name: 'label', type: 'String', variablesReference: 0 }; + Object.defineProperty(vscode.debug, 'activeDebugSession', { + configurable: true, + get: () => ({ type, customRequest: async (_command: string, args: { filter?: string }) => { + requests.push(args); + return { variables: args.filter === 'indexed' ? [ indexed ] : [ named ] }; + } }) + }); + try { + assert.deepStrictEqual(await new DebuggingExecutor().getVariableChildren(5, { indexedVariables: 1 }), + [ indexed, named ]); + assert.deepStrictEqual(requests, [ + { variablesReference: 5, filter: 'indexed', start: 0, count: 1 }, + { variablesReference: 5, filter: 'named' } + ]); + requests.length = 0; + await new DebuggingExecutor().getVariableChildren(5); + assert.deepStrictEqual(requests, [ { variablesReference: 5 } ]); + } finally { + Object.defineProperty(vscode.debug, 'activeDebugSession', descriptor); + } + }); + } + + test('non-Ruby String objects retain their children and metadata-like user fields', async () => { + const descriptor = Object.getOwnPropertyDescriptor(vscode.debug, 'activeStackItem')!; + Object.defineProperty(vscode.debug, 'activeStackItem', { + configurable: true, get: () => ({ frameId: 1, threadId: 1 }) + }); + const executor = { + hasActiveSession: async () => true, + getActiveSession: () => ({ type: 'pwa-node' }), + evaluateExpression: async () => ({ type: 'String', result: 'private preview', variablesReference: 1 }), + getVariableChildren: async () => [ + { name: '#class', type: 'string', value: 'private value', variablesReference: 0 }, + { name: '%ancestors', type: 'string', value: 'private value', variablesReference: 0 } + ] + } as unknown as IDebuggingExecutor; + try { + const output = await new DebuggingHandler(executor, {} as any, 30) + .handleEvaluateExpression({ expression: 'value' }); + assert.match(output, /#class \(string\)/); + assert.match(output, /%ancestors \(string\)/); + assert.doesNotMatch(output, /private/); + } finally { + Object.defineProperty(vscode.debug, 'activeStackItem', descriptor); + } + }); +}); diff --git a/src/test/rubyInspection.test.ts b/src/test/rubyInspection.test.ts new file mode 100644 index 0000000..505590a --- /dev/null +++ b/src/test/rubyInspection.test.ts @@ -0,0 +1,199 @@ +// Copyright (c) Microsoft Corporation. + +import * as assert from 'assert'; +import * as vscode from 'vscode'; +import { DebuggingHandler } from '../debuggingHandler'; +import { DebuggingExecutor, IDebuggingExecutor } from '../debuggingExecutor'; + +suite('Ruby rdbg variable inspection', () => { + function withActiveFrame(run: () => Promise): Promise { + const descriptor = Object.getOwnPropertyDescriptor(vscode.debug, 'activeStackItem'); + Object.defineProperty(vscode.debug, 'activeStackItem', { + configurable: true, + get: () => ({ frameId: 1, threadId: 1, session: {} }) + }); + return run().finally(() => { + if (descriptor) { + Object.defineProperty(vscode.debug, 'activeStackItem', descriptor); + } + }); + } + + test('returns scalar values instead of expanding rdbg metadata', async () => { + let expanded = false; + const executor = { + hasActiveSession: async () => true, + getActiveSession: () => ({ type: 'ruby_lsp' }), + getVariables: async () => ({ + scopes: [{ + name: 'Local variables', + variables: [{ + name: 'probe_value', + value: '41', + type: 'Integer', + variablesReference: 10 + }] + }] + }), + getVariableChildren: async () => { + expanded = true; + return [{ name: '#class', type: 'Class', variablesReference: 11 }]; + } + } as unknown as IDebuggingExecutor; + + await withActiveFrame(async () => { + const output = await new DebuggingHandler(executor, {} as any, 30) + .handleGetVariables({ variableNames: ['probe_value'], scope: 'local' }); + + assert.match(output, /probe_value: 41 \(Integer\)/); + assert.doesNotMatch(output, /#class/); + assert.strictEqual(expanded, false); + }); + }); + + test('keeps aggregate descendants to names and types without scalar metadata', async () => { + const expandedReferences: number[] = []; + const executor = { + hasActiveSession: async () => true, + getActiveSession: () => ({ type: 'ruby_lsp' }), + getVariables: async () => ({ + scopes: [{ + name: 'Local variables', + variables: [{ + name: 'probe_record', + value: '{:label=>"rbilling", :count=>41}', + type: 'Hash', + variablesReference: 20, + namedVariables: 3 + }] + }] + }), + getVariableChildren: async (reference: number, options: any) => { + expandedReferences.push(reference); + assert.deepStrictEqual(options, { indexedVariables: undefined }); + return [{ + name: '#class', + value: 'Hash', + type: 'Class', + variablesReference: 23 + }, { + name: ':label', + value: '"rbilling"', + type: 'String', + variablesReference: 21 + }, { + name: ':count', + value: '41', + type: 'Integer', + variablesReference: 22 + }]; + } + } as unknown as IDebuggingExecutor; + + await withActiveFrame(async () => { + const output = await new DebuggingHandler(executor, {} as any, 30) + .handleGetVariables({ variableNames: ['probe_record'], scope: 'local' }); + + assert.match(output, /probe_record \(Hash\)/); + assert.match(output, /:label \(String\)/); + assert.match(output, /:count \(Integer\)/); + assert.doesNotMatch(output, /rbilling|:count=>41|#class/); + assert.deepStrictEqual(expandedReferences, [20]); + }); + }); + + test('uses indexed DAP paging for Ruby Array children', async () => { + const requests: Array<{ command: string; args: any }> = []; + const descriptor = Object.getOwnPropertyDescriptor(vscode.debug, 'activeDebugSession'); + Object.defineProperty(vscode.debug, 'activeDebugSession', { + configurable: true, + get: () => ({ + id: 'ruby-session', + name: 'Ruby LSP', + type: 'ruby_lsp', + customRequest: async (command: string, args: any) => { + requests.push({ command, args }); + return { variables: [] }; + } + }) + }); + + try { + await new DebuggingExecutor().getVariableChildren(50, { + indexedVariables: 2 + }); + + assert.deepStrictEqual(requests, [{ + command: 'variables', + args: { + variablesReference: 50, + filter: 'indexed', + start: 0, + count: 2 + } + }, { + command: 'variables', + args: { variablesReference: 50, filter: 'named' } + }]); + } finally { + if (descriptor) { + Object.defineProperty(vscode.debug, 'activeDebugSession', descriptor); + } + } + }); + + test('returns scalar expression results instead of expanding rdbg metadata', async () => { + let expanded = false; + const executor = { + hasActiveSession: async () => true, + getActiveSession: () => ({ type: 'ruby_lsp' }), + evaluateExpression: async () => ({ + result: '42', + type: 'Integer', + variablesReference: 30 + }), + getVariableChildren: async () => { + expanded = true; + return [{ name: '#class', type: 'Class', variablesReference: 31 }]; + } + } as unknown as IDebuggingExecutor; + + await withActiveFrame(async () => { + const output = await new DebuggingHandler(executor, {} as any, 30) + .handleEvaluateExpression({ expression: 'probe_value + 1' }); + + assert.match(output, /Result: 42 \(Integer\)/); + assert.doesNotMatch(output, /#class/); + assert.strictEqual(expanded, false); + }); + }); + + test('still redacts Ruby String values that have metadata children', async () => { + const executor = { + hasActiveSession: async () => true, + getActiveSession: () => ({ type: 'ruby_lsp' }), + getVariables: async () => ({ + scopes: [{ + name: 'Local variables', + variables: [{ + name: 'api_token', + value: '"ghp_abcdefghijklmnopqrstuvwxyz0123456789"', + type: 'String', + variablesReference: 40 + }] + }] + }), + getVariableChildren: async () => { + throw new Error('Ruby String metadata should not be expanded'); + } + } as unknown as IDebuggingExecutor; + + await withActiveFrame(async () => { + const output = await new DebuggingHandler(executor, {} as any, 30) + .handleGetVariables({ variableNames: ['api_token'], scope: 'local' }); + + assert.doesNotMatch(output, /ghp_/); + assert.match(output, //); + }); + }); +});