diff --git a/CHANGELOG.md b/CHANGELOG.md index 3af513d..1217e4f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this ## [Unreleased] ### Fixed +- Preserve Ruby scalar values and retrieve both indexed and named children, with Ruby metadata filtering scoped to Ruby LSP. - Use Shopify Ruby LSP for Ruby file launches, passing the command and file separately. - Startup failures now report the configured pre-launch task and exit code when available, preserve configuration and test-dispatch errors, and direct agents to launch/task diagnostics instead of assuming a missing language extension. Readiness waits are cancelled when startup fails. diff --git a/docs/architecture/debuggingExecutor.md b/docs/architecture/debuggingExecutor.md index a819b1d..50a83c8 100644 --- a/docs/architecture/debuggingExecutor.md +++ b/docs/architecture/debuggingExecutor.md @@ -132,3 +132,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 0863c42..f5d55e3 100644 --- a/docs/architecture/debuggingHandler.md +++ b/docs/architecture/debuggingHandler.md @@ -110,3 +110,7 @@ operation finishes or fails. Test-dispatch failures propagate as errors rather than being interpreted as successful test completion. 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 c1eb067..c499067 100644 --- a/src/debuggingExecutor.ts +++ b/src/debuggingExecutor.ts @@ -24,6 +24,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; @@ -38,7 +42,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; @@ -556,7 +560,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 []; } @@ -567,9 +574,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 1f3a526..363c084 100644 --- a/src/debuggingHandler.ts +++ b/src/debuggingHandler.ts @@ -688,6 +688,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 @@ -828,7 +848,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 ? { @@ -846,7 +867,8 @@ export class DebuggingHandler implements IDebuggingHandler { ' ', 1, new Set(), - { remaining: this.maxExpandedFields } + { remaining: this.maxExpandedFields }, + response ); if (children.text) { resultText += `\n${children.text}`; @@ -895,7 +917,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)) { @@ -919,7 +942,8 @@ export class DebuggingHandler implements IDebuggingHandler { `${indent} `, depth + 1, visitedReferences, - expansionBudget + expansionBudget, + variable ); if (children.text) { text += `\n${children.text}`; @@ -935,7 +959,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 }; @@ -946,7 +971,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, //); + }); + }); +});