From d676d70a99364ca9da427230ebb6fe34c613cdde Mon Sep 17 00:00:00 2001 From: JF Date: Mon, 24 Aug 2026 19:42:05 -0400 Subject: [PATCH] fix(#468): get_local_variables walks down past an empty runtime top frame MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pause inside a blocking syscall/sleep puts a stdlib frame with no locals at stackFrames[0]; the tool returned an empty array even though the user frame's locals were one frame down and already fetched (the scope fan-out from #438 collects every frame). Extraction is now parameterized by anchor frame: when the top frame yields no locals, walk down to the first frame that does (policies anchor to the head of the frame list they receive, so slicing re-anchors them — zero extra DAP round-trips). The response's `frame` names the anchored frame and a `note` discloses the walk-down. Skipped under an explicit `names` filter, where "nothing matched in the top frame" is the honest answer (notFound reports it). The Windows live repro (cpp pause) anchors to LLDB's injected DbgBreakPoint *thread*, which is the sibling issue #465's shape — thread-level adoption lands there and composes with this fix. Co-Authored-By: Claude Fable 5 --- src/server.ts | 9 +- src/session/session-manager-data.ts | 94 ++++++++++++------ .../unit/session/session-manager-dap.test.ts | 98 +++++++++++++++++++ 3 files changed, 170 insertions(+), 31 deletions(-) diff --git a/src/server.ts b/src/server.ts index c4dbf85b..c9677b59 100644 --- a/src/server.ts +++ b/src/server.ts @@ -890,6 +890,7 @@ export class DebugMcpServer { variables: Variable[]; frame: { name: string; file: string; line: number } | null; scopeName: string | null; + anchorNote?: string; truncation?: VariableTruncationSummary; }> { this.validateSession(sessionId); @@ -2730,12 +2731,18 @@ export class DebugMcpServer { if (result.frame) { response.frame = result.frame; } - + // Include scope name if available if (result.scopeName) { response.scopeName = result.scopeName; } + // The tool walked down past an empty runtime/stdlib top frame — say so, + // since `frame` no longer names the top of the stack (issue #468). + if (result.anchorNote) { + response.note = result.anchorNote; + } + // Surface adapter warnings embedded in the scope name — e.g. Delve // reports "Locals (warning: optimized function)" when the debuggee was // built with optimizations, which typically means missing variables. diff --git a/src/session/session-manager-data.ts b/src/session/session-manager-data.ts index b0e861d3..c3801f53 100644 --- a/src/session/session-manager-data.ts +++ b/src/session/session-manager-data.ts @@ -410,6 +410,8 @@ export abstract class SessionManagerData extends SessionManagerCore { variables: Variable[]; frame: { name: string; file: string; line: number } | null; scopeName: string | null; + /** Set when the top frame had no locals and a lower frame was anchored instead (issue #468). */ + anchorNote?: string; truncation?: VariableTruncationSummary; }> { const session = this._getSessionById(sessionId); @@ -482,35 +484,60 @@ export abstract class SessionManagerData extends SessionManagerCore { // Step 4: Get the appropriate adapter policy const policy = this.selectPolicy(session.language); - - // Step 5: Extract local variables using the adapter policy - let localVars: Variable[] = []; - let scopeName: string | null = null; - - if (policy.extractLocalVariables) { - localVars = policy.extractLocalVariables(stackFrames, scopesMap, variablesMap, includeSpecial); - - // Report the ACTUAL scope name the adapter returned, not the policy's - // canonical name — adapters may annotate it (e.g. Delve's "Locals - // (warning: optimized function)") and that annotation matters to the - // caller. Fall back to the canonical name when no scope matches. - const canonicalNames = policy.getLocalScopeName - ? ([] as string[]).concat(policy.getLocalScopeName()) - : []; - const topFrameScopes = scopesMap[topFrame.id] || []; - const matchedScope = topFrameScopes.find(s => - canonicalNames.some(c => s.name === c || s.name.startsWith(c + ' ')) - ); - scopeName = matchedScope?.name ?? canonicalNames[0] ?? null; - } else { - // Fallback: use first non-global scope from top frame - const topFrameScopes = scopesMap[topFrame.id] || []; - const localScope = topFrameScopes.find(s => !s.name.toLowerCase().includes('global')); - if (localScope) { - localVars = variablesMap[localScope.variablesReference] || []; - scopeName = localScope.name; + + // Step 5: Extract local variables using the adapter policy. Policies + // anchor to the first frame of the list they receive, so extraction is + // parameterized by anchor: slicing the frame list re-anchors it. + const extractAt = (frames: StackFrame[]): { localVars: Variable[]; scopeName: string | null } => { + const anchor = frames[0]; + if (policy.extractLocalVariables) { + const vars = policy.extractLocalVariables(frames, scopesMap, variablesMap, includeSpecial); + + // Report the ACTUAL scope name the adapter returned, not the policy's + // canonical name — adapters may annotate it (e.g. Delve's "Locals + // (warning: optimized function)") and that annotation matters to the + // caller. Fall back to the canonical name when no scope matches. + const canonicalNames = policy.getLocalScopeName + ? ([] as string[]).concat(policy.getLocalScopeName()) + : []; + const anchorScopes = scopesMap[anchor.id] || []; + const matchedScope = anchorScopes.find(s => + canonicalNames.some(c => s.name === c || s.name.startsWith(c + ' ')) + ); + return { localVars: vars, scopeName: matchedScope?.name ?? canonicalNames[0] ?? null }; + } + // Fallback: use first non-global scope from the anchor frame + const anchorScopes = scopesMap[anchor.id] || []; + const localScope = anchorScopes.find(s => !s.name.toLowerCase().includes('global')); + return localScope + ? { localVars: variablesMap[localScope.variablesReference] || [], scopeName: localScope.name } + : { localVars: [], scopeName: null }; + }; + + let anchorIndex = 0; + let { localVars, scopeName } = extractAt(stackFrames); + + // A pause inside a runtime/stdlib frame (blocking syscall, sleep) puts + // an empty-locals frame on top while the user frame sits just below — + // and its scopes are already fetched. Walk down to the first frame + // that yields locals rather than returning an empty result the caller + // cannot act on (issue #468). Skipped under an explicit `names` filter, + // where "nothing matched in the top frame" is the honest answer. + if (localVars.length === 0 && names === undefined) { + for (let k = 1; k < stackFrames.length; k++) { + const attempt = extractAt(stackFrames.slice(k)); + if (attempt.localVars.length > 0) { + anchorIndex = k; + localVars = attempt.localVars; + scopeName = attempt.scopeName; + this.logger.info( + `[SM getLocalVariables ${sessionId}] Top frame '${topFrame.name}' had no locals; anchored to frame #${k} '${stackFrames[k].name}'.` + ); + break; + } } } + const anchorFrame = stackFrames[anchorIndex]; // Attribute per-scope truncation to the scopes whose variables // actually reached the caller (issue #438): every policy returns @@ -547,11 +574,18 @@ export abstract class SessionManagerData extends SessionManagerCore { return { variables: cappedLocals.variables, frame: { - name: topFrame.name, - file: topFrame.file, - line: topFrame.line + name: anchorFrame.name, + file: anchorFrame.file, + line: anchorFrame.line }, scopeName, + ...(anchorIndex > 0 + ? { + anchorNote: + `Top frame '${topFrame.name}' has no local variables (runtime/stdlib frame); ` + + `showing frame #${anchorIndex} '${anchorFrame.name}' instead` + } + : {}), ...(truncation ? { truncation } : {}) }; diff --git a/tests/core/unit/session/session-manager-dap.test.ts b/tests/core/unit/session/session-manager-dap.test.ts index c6c9e7b5..d96ad143 100644 --- a/tests/core/unit/session/session-manager-dap.test.ts +++ b/tests/core/unit/session/session-manager-dap.test.ts @@ -956,6 +956,104 @@ describe('SessionManager - DAP Operations', () => { ); }); + it('walks down past an empty runtime top frame to the first frame with locals (issue #468)', async () => { + const session = await createPausedSession(); + + // A pause inside a blocking sleep: frame 0 is a stdlib frame with an + // empty Local scope; the user frame with the loop counter is frame 1. + dependencies.mockProxyManager.sendDapRequest = vi.fn().mockImplementation( + async (command: string, args?: { frameId?: number; variablesReference?: number }) => { + switch (command) { + case 'stackTrace': + return { + success: true, + body: { + stackFrames: [ + { id: 1, name: 'std::this_thread::sleep_for', source: { path: '/usr/include/c++/13/bits/this_thread_sleep.h' }, line: 80, column: 0 }, + { id: 2, name: 'main', source: { path: '/proj/examples/cpp/pause_test.cpp' }, line: 20, column: 0 } + ] + } + }; + case 'scopes': + return { + success: true, + body: { + scopes: [{ name: 'Local', variablesReference: args?.frameId === 2 ? 200 : 100, expensive: false }] + } + }; + case 'variables': + return { + success: true, + body: { + variables: args?.variablesReference === 200 + ? [{ name: 'counter', value: '348', type: 'long long', variablesReference: 0 }] + : [] + } + }; + default: + return { success: true }; + } + } + ); + + const result = await sessionManager.getLocalVariables(session.id); + + expect(result.variables).toEqual([ + expect.objectContaining({ name: 'counter', value: '348' }) + ]); + // The response must disclose the anchor walked down from the top frame. + expect(result.frame).toEqual(expect.objectContaining({ name: 'main', line: 20 })); + expect(result.anchorNote).toMatch(/sleep_for/); + expect(result.anchorNote).toMatch(/'main'/); + }); + + it('does not walk down when an explicit names filter is set (issue #468)', async () => { + const session = await createPausedSession(); + + dependencies.mockProxyManager.sendDapRequest = vi.fn().mockImplementation( + async (command: string, args?: { frameId?: number; variablesReference?: number }) => { + switch (command) { + case 'stackTrace': + return { + success: true, + body: { + stackFrames: [ + { id: 1, name: 'std::this_thread::sleep_for', source: { path: '/usr/include/c++/13/bits/this_thread_sleep.h' }, line: 80, column: 0 }, + { id: 2, name: 'main', source: { path: '/proj/examples/cpp/pause_test.cpp' }, line: 20, column: 0 } + ] + } + }; + case 'scopes': + return { + success: true, + body: { + scopes: [{ name: 'Local', variablesReference: args?.frameId === 2 ? 200 : 100, expensive: false }] + } + }; + case 'variables': + return { + success: true, + body: { + variables: args?.variablesReference === 200 + ? [{ name: 'counter', value: '348', type: 'long long', variablesReference: 0 }] + : [] + } + }; + default: + return { success: true }; + } + } + ); + + // "counter" exists one frame down, but a names request is scoped to the + // top frame — the honest answer is that nothing matched there. + const result = await sessionManager.getLocalVariables(session.id, false, ['counter']); + + expect(result.variables).toEqual([]); + expect(result.frame).toEqual(expect.objectContaining({ name: 'std::this_thread::sleep_for' })); + expect(result.anchorNote).toBeUndefined(); + }); + it('caps an enormous scope and reports truncation instead of blowing the response (issues #356/#359)', async () => { const session = await createPausedSession();